Fix remaining barriers to loading @apollo/client/core as native ES modules from a CDN
#8266
Assignees
Projects
Comments
benjamn
added a commit
that referenced
this issue
Jun 3, 2021
As explained in #8266, our use of process.env.NODE_ENV requires those expressions to be either replaced with a string literal by a minifier (common convention in the React ecosystem) or polyfilled globally. We stopped polyfilling process.env globally in the ts-invariant package in apollographql/invariant-packages#94, but @apollo/client is still relying on process.env internally, as is the graphql package. If we want to rid ourselves fully of the drawbacks of process.env.NODE_ENV, we probably ought to stop using it ourselves. Though most developers in the React ecosystem will be using a bundler or minifier that replaces process.env.NODE_ENV at build time, you may not have the luxury of custom minification when loading @apollo/client from a CDN, which leaves only the option of a global process polyfill, which is problematic because that's how some applications detect if the current code is running Node.js (more details/links in #8266). Instead, I believe we can (and must?) stop using process.env.NODE_ENV, and one of many better alternatives appears to be the __DEV__ variable used by React Native, which is much easier to polyfill, since it's a single variable rather than a nested object. Switching to __DEV__ will initially cause a large bundle size regression (+3.5Kb *after* minification and gzip), but that's not technically a breaking change (and thus acceptable for AC3.4), and it should be easy to reconfigure minifiers to replace __DEV__ with true or false (or even just undefined), with sufficient guidance from the release notes. That still leaves the process.env.NODE_ENV check in instanceOf.mjs in the graphql package. Discussion in graphql/graphql-js#2894 suggests the plan is to stop using NODE_ENV altogether, which would be ideal, but that won't happen until graphql@16 at the earliest. To work around the problem in the meantime, I devised a strategy where we polyfill process.env.NODE_ENV only briefly, while we import code that depends on graphql/jsutils/instanceOf.mjs, and then synchronously remove the global polyfill, so it does not permanently pollute the global namespace. This strategy assumes @apollo/client is the first to import the graphql package. If you have other code that imports instanceOf.mjs earlier, and you don't have a process.env.NODE_ENV strategy already, it's your responsibility to make that import work, however you see fit. Apollo Client is only responsible for making sure its own imports of the graphql package have a chance of succeeding, a responsibility I believe my strategy fulfills cleverly if not elegantly. Of course, this charade does not happen if process.env.NODE_ENV is already defined or has been minified away, but only if accessing it would throw, since that's what we're trying to avoid. Although we could do some more work to reduce the bundle size impact of blindly switching to __DEV__, I believe this PR already solves the last remaining hurdles documented in #8266, potentially allowing @apollo/client/core@beta to be loaded from an ESM-aware CDN like esm.run or jspm.io. The default __DEV__ value will be true in those environments, but could be initialized differently by a script/module that runs earlier in the HTML of the page.
benjamn
added a commit
that referenced
this issue
Jun 4, 2021
As explained in #8266, our use of process.env.NODE_ENV requires those expressions to be either replaced with a string literal by a minifier (common convention in the React ecosystem) or polyfilled globally. We stopped polyfilling process.env globally in the ts-invariant package in apollographql/invariant-packages#94, but @apollo/client is still relying on process.env internally, as is the graphql package. If we want to rid ourselves fully of the drawbacks of process.env.NODE_ENV, we probably ought to stop using it ourselves. Though most developers in the React ecosystem will be using a bundler or minifier that replaces process.env.NODE_ENV at build time, you may not have the luxury of custom minification when loading @apollo/client from a CDN, which leaves only the option of a global process polyfill, which is problematic because that's how some applications detect if the current code is running Node.js (more details/links in #8266). Instead, I believe we can (and must?) stop using process.env.NODE_ENV, and one of many better alternatives appears to be the __DEV__ variable used by React Native, which is much easier to polyfill, since it's a single variable rather than a nested object. Switching to __DEV__ will initially cause a large bundle size regression (+3.5Kb *after* minification and gzip), but that's not technically a breaking change (and thus acceptable for AC3.4), and it should be easy to reconfigure minifiers to replace __DEV__ with true or false (or even just undefined), with sufficient guidance from the release notes. That still leaves the process.env.NODE_ENV check in instanceOf.mjs in the graphql package. Discussion in graphql/graphql-js#2894 suggests the plan is to stop using NODE_ENV altogether, which would be ideal, but that won't happen until graphql@16 at the earliest. To work around the problem in the meantime, I devised a strategy where we polyfill process.env.NODE_ENV only briefly, while we import code that depends on graphql/jsutils/instanceOf.mjs, and then synchronously remove the global polyfill, so it does not permanently pollute the global namespace. This strategy assumes @apollo/client is the first to import the graphql package. If you have other code that imports instanceOf.mjs earlier, and you don't have a process.env.NODE_ENV strategy already, it's your responsibility to make that import work, however you see fit. Apollo Client is only responsible for making sure its own imports of the graphql package have a chance of succeeding, a responsibility I believe my strategy fulfills cleverly if not elegantly. Of course, this charade does not happen if process.env.NODE_ENV is already defined or has been minified away, but only if accessing it would throw, since that's what we're trying to avoid. Although we could do some more work to reduce the bundle size impact of blindly switching to __DEV__, I believe this PR already solves the last remaining hurdles documented in #8266, potentially allowing @apollo/client/core@beta to be loaded from an ESM-aware CDN like esm.run or jspm.io. The default __DEV__ value will be true in those environments, but could be initialized differently by a script/module that runs earlier in the HTML of the page.
benjamn
changed the title
@apollo/client/core as native ES modules from a CDN
|
For anyone who might be following along, I'm excited to break the news you can now import // Latest beta version:
await import("https://esm.run/@apollo/client@beta/core")
// Using an explicit version:
await import("https://esm.run/@apollo/client@3.4.0-rc.10/core")
// Full redirected URL:
await import("https://cdn.jsdelivr.net/npm/@apollo/client@3.4.0-rc.10/core/+esm")in your browser console, or by embedding a <script type=module>
import {
ApolloClient,
InMemoryCache,
} from "https://esm.run/@apollo/client@beta/core";
const client = new ApolloClient({
cache: new InMemoryCache,
});
console.log(client);
</script>in any webpage, all without any local build steps (tested in Chrome, Firefox, and Safari)!
This milestone took several different kinds of work, from wrapping Time will tell if this way of loading |
2 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
In other words, the following dynamic
imports should succeed in modern web browsers without errors:Since the goal here is to load native ECMAScript modules, not UMD or CommonJS bundles, all npm dependencies of
@apollo/client/coremust export ECMAScript modules for the aboveimport(...)to succeed. See #8222 (comment) for background on several packages that violated this requirement, but that we have fixed/removed/replaced. Progress!Another remaining issue is that our
ts-invariantdependency was previously polyfillingglobalThis.process, but stopped (for good reasons) in apollographql/invariant-packages#94, which was released ints-invariant@0.7.0, which will be required by@apollo/client@3.4.0.While many bundlers/minifiers replace
process.env.NODE_ENVat build time with a string literal like"production", there are a number of libraries, including@apollo/client,graphql, andreact, that publish code containing thoseprocess.envexpressions, expecting them either to be stripped or polyfilled. Neither expectation is met when these packages' code is loaded from an ESM-aware CDN like https://esm.run, so the first unguardedprocessreference throws a fatalReferenceError.We are actively looking for a good alternative approach (tracking issue: #8187), but in the meantime it seems important for backwards compatibility to continue polyfilling
globalThis.process.envuntil Apollo Client 4, by which time hopefully thegraphqlpackage will have removed its (single!) use ofprocess.env.NODE_ENV(ininstanceOf.mjs).While we're at it,
@apollo/clientshould make its own usages ofprocess.envmore defensive about running in environments without a globalprocessobject and no build step to remove those expressions.Related: #7895, reported by @abdonrd
The text was updated successfully, but these errors were encountered: