Skip to content
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

Enforce that __DEV__ is polyfilled in every entry point that uses it #8689

Merged
merged 4 commits into from Aug 24, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Aug 23, 2021

This PR started out as a quick fix for #8674, but in the process I realized the best way to prevent ReferenceErrors for __DEV__ once and for all is to write a build step that examines compiled dist/ code for usage of __DEV__, and enforces that the corresponding entry point index.js file imports and calls checkDEV().

To test these changes using a published version of @apollo/client, I merged this PR branch into release-3.5 and published @apollo/client@3.5.0-beta.7 with this PR included. From my testing, this version works as before with ESM-aware CDNs like esm.run. In particular, the following code works in a browser console:

await import("https://esm.run/@apollo/client@3.5.0-beta.7/core")

See #8266 for more background on why this matters here.

Related issues and PRs:

@benjamn benjamn self-assigned this Aug 23, 2021
Comment on lines +1 to +2
import { checkDEV } from "../../utilities";
checkDEV();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out a number of entry points that weren't obviously using __DEV__ actually do need it, because they use invariant or InvariantError, which we transform to an expression containing __DEV__ to enable stripping long error messages in production.

Comment on lines +7 to +8
export function checkDEV() {
invariant("boolean" === typeof DEV, DEV);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checkDEV function not only checks that the DEV.ts module successfully exported a boolean value (DEV), but also indirectly ensures __DEV__ actually works, because this invariant(...) expression is transformed to __DEV__ ? invariant(...) : invariant(...) during build.

benjamn added a commit that referenced this pull request Aug 23, 2021
This merge of the still-open PR #8689 into `release-3.5` will allow us
to test those changes in an `@apollo/client@beta` release before merging
to `main` and releasing in v3.4.x.
benjamn added a commit that referenced this pull request Aug 23, 2021
This merge of the still-open PR #8689 into `release-3.5` will allow us
to test those changes in an `@apollo/client@beta` release before merging
to `main` and releasing in v3.4.x.
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix @benjamn - thanks! 🎉

@benjamn benjamn merged commit 6e2b51d into main Aug 24, 2021
@benjamn benjamn deleted the issue-8674-enforce-__DEV__-everywhere branch August 24, 2021 14:32
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ReferenceError: __DEV__ is not defined after update to 3.4.5 with Next.js app
2 participants