@apollo/client v3 broken in Rollup & Snowpack

[FredKSchott]

Tracked it back to this item:

https://github.com/apollographql/apollo-client/blob/master/src/react/react.ts#L8

The require function doesn't exist inside an ES Module (any file with import or export syntax), which is causing the package to fail in some bundlers.

Happy to help brainstorm ideas on how to solve if you'd like, but would strongly recommend against shipping 3.0 with broken support in any major bundlers.

[FredKSchott]

related: #6035

[hwillson]

@FredKSchott can you post an example of the failures you're seeing? I haven't tried Snowpack, but AC3 definitely works with Rollup. Thanks!

[benjamn]

Good catch @FredKSchott.

It's disappointing that the only way to defer an ESM import is to use dynamic import(), which introduces asynchrony into the codebase, and is implemented differently by just about every major JS bundler/runtime.

Please do give us your thoughts on how we might avoid importing React when it isn't needed!

[FredKSchott]

Yup, 100% agreed and wish this was better supported in ESM. But even Webpack is limited by how much it can statically analyze. Bundlers can see that @apollo/client is importing & requiring React, but they can't tell at build-time wether that runtime function that does it will or won't ever be run, so a bundler will normally just bundle "react" into your app just to be safe.

Based on the above, I'm 99% sure that Webpack is bundling "react" into every application that uses @apollo/client v3 regardless of whether the that code path ever gets hit. I don't think it's actually buying you that much.

I'd recommend:

1. Decide that @apollo/client is for React, and remove the layer of indirection / just import react directly. Webpack is already including React anyway, so this isn't too different from the way it is behaving today, but you'd be able to support Rollup, Snowpack, etc, etc.
2. Split out anything React-specific from @apollo/client into a new @apollo/react or @apollo/client-react. I'd actually recommend this regardless of this issue, since it would help you scale to @apollo/client-{FRAMEWORK} without much thrash, but that's just my outsiders opinion.
3. Move anything React-specific into a @apollo/client/react sub-path within the package, and have users import from there directly. Everyone who npm installs the package will still get react as a dependency, but if the user never imports @apollo/client/react in their project, bundlers should be smart enough to handle that.

[hwillson]

Thanks @FredKSchott, we'll look into this further. Just a small note though - AC3's current lazy react require approach does work with Rollup. You can see it in action here.

[ashubham]

@hwillson FWIW, the example you are citing is no-react, where the lazy require is not even needed.

I did a workaround though to make it work with Rollup/React:

I created a rollup plugin which replaces the lazy require with a static import.

export function patchApolloReactRequire() {
    return {
        name: 'patch-apollo-react-require',
        transform(code, id) {
            if (id.endsWith('@apollo/client/react/react.js')) {
                return `import React from 'react';export function requireReactLazily(){return React}`;
            }
        }
    }
}

[FredKSchott]

Yup, that's correct, the lazy require isn't called in that example. ApolloProvider is an example of something that calls the broken lazy require

[abdonrd]

2. Split out anything React-specific from @apollo/client into a new @apollo/react or @apollo/client-react. I'd actually recommend this regardless of this issue, since it would help you scale to @apollo/client-{FRAMEWORK} without much thrash, but that's just my outsiders opinion.

I really like the second option from @FredKSchott!

[abdonrd]

Also about ES Modules related, @apollo/client still uses three dependencies exported as CommonJS:

• graphql-tag (Use graphql-tag as ESM #6074)
• zen-observable (Use zen-observable as ESM #5961)
• fast-json-stable-stringify

I have noticed this trying to make @apollo/client work directly in the browser, without using any build/bundled process during development.

[ashubham]

There is a 4th way too, which can achieve conditional bundling without asking users to use a different namespace to import from:

Have 2 modules:

// src/react/react.ts

let React: typeof import('react');

export function requireReactLazily(): typeof import('react') {
  if(!React) {
    throw new Error('React not loaded, consider putting `import @apollo/client/react/load`');
  }
  return React;
}

export function setReact(_React) {
  React = _React;
}

And another,

// src/react/load.ts

import React from 'react';
import { setReact } from './react';

setReact(React);

Now, have the users when using apollo-client in a react app do:

// app.tsx

import { ApolloClient, ApolloProvider, useQuery } from '@apollo/client';
import '@apollo/client/react/load'; // This is required only once.

... Rest of the App ...

Instead of load.ts if we name it index.ts, we can have users just import '@apollo/client/react'.

[hwillson]

Thanks all - sorry, I misinterpreted the original problem regarding the example app I shared (I was focused on the use case of not needing React).

We had discussed a lot of the ideas in this thread back when we decided to integrate our React hooks into the @apollo/client core. The thing is, we're very focused on providing an as easy to use out of the box Apollo Client + React experience as possible. React is the only view layer we officially support ourselves, which means it gets higher priority in our design decisions and usability goals. We'll put some more thought into this, but the most likely outcome here (for the short term) is that we'll just remove the lazy load / require for the AC 3.0 release. We'll then investigate a way to make React more detachable, but that method will not be part of the default out of the box @apollo/client package install experience. TBD though, more details shortly.

[FredKSchott]

@hwillson I think that's a good call 👍

[FredKSchott]

Excellent! Thanks for the quick fix. What's the best way to test this in Snowpack/Rollup?

[hwillson]

@FredKSchott we'll publish a new beta shortly (I'll post back here when it's ready). Thanks!

[hwillson]

@FredKSchott we just published @apollo/client@3.0.0-beta.53 which should hopefully fix this 🤞.

[FredKSchott]

That worked! Thanks again