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

Fix remaining barriers to loading @apollo/client/core as native ES modules from a CDN #8266

Closed
benjamn opened this issue May 19, 2021 · 1 comment · Fixed by #8347
Closed

Comments

@benjamn
Copy link
Member

benjamn commented May 19, 2021

In other words, the following dynamic imports should succeed in modern web browsers without errors:

import("https://cdn.jsdelivr.net/npm/@apollo/client@beta/core/+esm")
import("https://esm.run/@apollo/client@beta/core")

Since the goal here is to load native ECMAScript modules, not UMD or CommonJS bundles, all npm dependencies of @apollo/client/core must export ECMAScript modules for the above import(...) 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-invariant dependency was previously polyfilling globalThis.process, but stopped (for good reasons) in apollographql/invariant-packages#94, which was released in ts-invariant@0.7.0, which will be required by @apollo/client@3.4.0.

While many bundlers/minifiers replace process.env.NODE_ENV at build time with a string literal like "production", there are a number of libraries, including @apollo/client, graphql, and react, that publish code containing those process.env expressions, 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 unguarded process reference throws a fatal ReferenceError.

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.env until Apollo Client 4, by which time hopefully the graphql package will have removed its (single!) use of process.env.NODE_ENV (in instanceOf.mjs).

While we're at it, @apollo/client should make its own usages of process.env more defensive about running in environments without a global process object and no build step to remove those expressions.

Related: #7895, reported by @abdonrd

@benjamn benjamn added this to the Release 3.4 milestone May 19, 2021
@benjamn benjamn self-assigned this May 19, 2021
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 benjamn linked a pull request Jun 4, 2021 that will close this issue
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 benjamn changed the title Fix remaining barriers to loading @apollo/client/core as native ES modules from a CDN Fix remaining barriers to loading @apollo/client/core as native ES modules from a CDN Jun 16, 2021
@benjamn
Copy link
Member Author

benjamn commented Jun 16, 2021

For anyone who might be following along, I'm excited to break the news you can now import ApolloClient either by evaluating any of the following expressions

// 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> element like

<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)!

Note: @apollo/client/core instead of @apollo/client, because @apollo/client depends (regrettably) on react and react-dom, which do not export ECMAScript modules (see #8190 for more details).

This milestone took several different kinds of work, from wrapping zen-observable with zen-observable-ts (#7615), to converting graphql-tag to a TypeScript project (apollographql/graphql-tag#325), to replacing fast-json-stable-stringify with canonicalStringify (#8222), to switching from process.env.NODE_ENV to __DEV__ (#8347), while at the same time making sure the process.env usage within the graphql package doesn't throw runtime errors in buildless environments (apollographql/invariant-packages#139), a hack I would be happy to unwind if the graphql maintainers would merge this PR, or if they would join us in no longer using process.env.NODE_ENV at all (in graphql@16).

Time will tell if this way of loading @apollo/client is truly viable in production, compared to application-level bundling, but it seems pretty convenient for experimentation, at least! Please let us know how it works for you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.