-
Notifications
You must be signed in to change notification settings - Fork 34
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
remove process.env.NODE_ENV
warning by bumping @opennextjs/aws
dependency
#211
Conversation
🦋 Changeset detectedLatest commit: c22478b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
9c97a02
to
b5b7994
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.
It looks like the right (safe) thing to do but because this is Open Next code, it would be easier to modify the code at https://github.com/opennextjs/opennextjs-aws/blob/9595714ac23e5f131b879d04d5cfb2a5d11bdbdd/packages/open-next/src/adapters/util.ts#L4
yes I agree, but I am not sure how to discern there if the code is being used by Cloudflare and also I wouldn't want to overcomplicate things there only for our use case, I think that it's probably something worth tackling/looking into when we move to a shared monorepo setup, right now I would just add this quick patch to remove the warning not to annoy/scare users away but I agree that ideally it should be solved in a better way, I just don't think that it's a high enough priority right for us to tackle in such a way right now if you disagree with the patch I am totally fine not including it 👍 |
Yes the line I pointed to is the one causing the warning. It is better to make the function 2 lines there to avoid adding a patch that we would migrate to AST later. |
|
b5b7994
to
c22478b
Compare
@vicb PR updated to only bump the aws dependency |
process.env.NODE_ENV
warningprocess.env.NODE_ENV
warning by bumping @opennextjs/aws
dependency
A quick (potentially temporary) solution for the

process.env.NODE_ENV
warning:This change consists in simply bumping the aws dependency to its prerelease package generated in opennextjs/opennextjs-aws#686 (comment)