-
Notifications
You must be signed in to change notification settings - Fork 930
__DEV__
logic is now ‘NODE_ENV is not production’
#2907
__DEV__
logic is now ‘NODE_ENV is not production’
#2907
Conversation
🦋 Changeset detectedLatest commit: b9f7231 The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 packages
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @steveluscher and the rest of your teammates on Graphite |
cc/ @Jac0xb |
626886e
to
c1f70fe
Compare
This mirrors, for instance, React's behaviour. Their library uses |
c1f70fe
to
d90c030
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!
await expect(subscribePromise).rejects.toThrow(new SolanaError(123 as SolanaErrorCode, undefined)); | ||
await expect(subscribePromise).rejects.toThrow( | ||
new SolanaError(SOLANA_ERROR__SUBTLE_CRYPTO__DIGEST_UNIMPLEMENTED), | ||
); |
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.
CI error seems to be related to the changes in this file - are they intended to be in this PR at all?
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.
Hmm. This change was supposed to fix a test failure owing to the fact that error 123 doesn’t exist.
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.
Oh, it’s lint I left behind.
Maybe I’ll try to do this properly with mocks this time.
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.
Thanks, I'll make sure I replicate on Kinobi when I'm out of this documentation hole.
d90c030
to
b9f7231
Compare
🎉 This PR is included in version 1.95.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
Summary
A few folks have been caught out not having any setting for
NODE_ENV
. In cases like this, the__DEV__
flag will befalse
becauseNODE_ENV
is not'development'
.It would be preferable for development to be the default when there is no
NODE_ENV
. This means it will be harder to get ‘into’ production mode (you have to set an env variable) but it will mean that people fall into the pit of success when debug functionality is what they're looking for.Test Plan
Observe that all of the
__DEV__
constants have been replaced withprocess.env.NODE_ENV !== 'production'