-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change setNodeEnv
code to avoid warnings when using the Cloudflare adapter
#686
Conversation
|
ba5fef3
to
060921e
Compare
commit: |
Coverage Report
File Coverage
|
060921e
to
716436e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Not a big fan of this change either to be honest. We need to find a better way of defining this (or delegate it to the platform itself).
Order of import can cause this issue #624
thanks 🙂 yeah I agree it does feel like this could be improved 😕 |
This is a quick change to avoid warnings such as:
when using the Cloudflare adapter
I am not a huge fan of this change but, for a quick fix which doesn't require too much refactoring in the aws package, I think it's either this or patching the aws code on the Cloudflare adapter (as I've done in opennextjs/opennextjs-cloudflare#211).
(A more proper fix would be to make the aws code configurable so that when used by the Cloudflare adapter the
setNodeEnv
is either never called, thus three shaken away, or becomes a no-op function, but that as I referred to before would be a much bigger change, which might not be worth it right now)