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

Add Node/Browser ESM support to @apollo/client. #287

Open
brainkim opened this issue Mar 24, 2021 · 23 comments
Open

Add Node/Browser ESM support to @apollo/client. #287

brainkim opened this issue Mar 24, 2021 · 23 comments
Labels
core Feature requests related to core functionality

Comments

@brainkim
Copy link

brainkim commented Mar 24, 2021

This is a tracking issue for the work to add native ESM support for new Node versions and direct <script type="module"> references for the @apollo/client package.

This would probably mean adding "type": "module" to package.json files and potentially exports fields. It may also include changes to the build system and package artifacts. It is potentially a breaking change for unwitting developers who have imported files which we never intended to export.

Maybe this is a low priority, especially given most bundlers and front-end tools conflate and handle CommonJS and ESModules out of box, and I’m not sure what percentage of users import apollo-client from ESM-capable node versions. Maybe someone doing SSR?

Also note that this is a risky change to the codebase which will cause outages if we don’t dot our t’s and cross our i’s. See https://javascript.plainenglish.io/is-promise-post-mortem-cab807f18dcc, for instance.

@benjamn @hwillson @jcreighton

Related issues:
#81 #83

@JanNitschke
Copy link

This could also be accomplished by switching all generated module files in the dist folder to the mjs file extension and changing "module": "index.js" to "module": "index.mjs" in the package.json file. This lets node import these files directly and should not break any build tools but would beak all import specifying the file extension:

import { ApolloClient } from '@apollo/client/core/ApolloClient.js';

This could be fixed by keeping the .js files and duplicating them with the .mjs extension.

I used this to get Apollo working in a esbuild project that only works with native ESM.

@eric-burel
Copy link

eric-burel commented Mar 12, 2022

Hi, this is problematic for us at Vulcan since we have moved our package library, that depends on @apollo/client, to ESM.
Our library won't work in Next and users are facing similar issues in Svelte or Nuxt.

This StackOverflow question sums it up: https://stackoverflow.com/questions/70615613/apollo-client-named-export-remove-not-found
We have the same issue with gql, useQuery etc. with no solution yet except maybe writing a custom Esbuild plugin to alter the import.

Next.js, which is based on Webpack 5, doesn't seem to support just having "module": "index.js", exports are needed instead. I could fix most issues by adding this to my @apollo/client package.json:

"exports": {
    ".": "./index.js",
    "./link/error": "./link/error/index.js"
  },

And also in ts-invariant package.json:

  "type": "module",
  "exports": {
    ".": "./lib/invariant.esm.js",
    "./process/index.js": "./process/index.js"
  },

And finally to ts-invariant/process package.json:

  "type": "module",

@eric-burel
Copy link

Here is a gist: https://gist.github.com/eric-burel/543bdc809bcc62208deabdeb004db724
Tested with Apollo 3.5.10
Add node ./fix-apollo.js to postinstall package.json

@fabis94
Copy link

fabis94 commented Apr 14, 2022

Any updates here? A year later it's still an issue when trying to use @apollo/client with something like Nuxt on Node 16

file:///home/fabis/Code/speckle/speckle-website-fe/node_modules/@apollo/client/utilities/globals/fix-graphql.js:1
import { remove } from "ts-invariant/process/index.js";
         ^^^^^^
SyntaxError: Named export 'remove' not found. The requested module 'ts-invariant/process/index.js' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'ts-invariant/process/index.js';
const { remove } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:127:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:191:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:337:24)
    at async __instantiateModule__ (file:///home/fabis/Code/speckle/speckle-website-fe/.nuxt/dist/server/server.mjs:23402:3)

@eric-burel
Copy link

Any updates here? A year later it's still an issue when trying to use @apollo/client with something like Nuxt on Node 16

file:///home/fabis/Code/speckle/speckle-website-fe/node_modules/@apollo/client/utilities/globals/fix-graphql.js:1
import { remove } from "ts-invariant/process/index.js";
         ^^^^^^
SyntaxError: Named export 'remove' not found. The requested module 'ts-invariant/process/index.js' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'ts-invariant/process/index.js';
const { remove } = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:127:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:191:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:337:24)
    at async __instantiateModule__ (file:///home/fabis/Code/speckle/speckle-website-fe/.nuxt/dist/server/server.mjs:23402:3)

It seems that exports map is not an option for Apollo because it would break older applications, so the best hope seems to figure out why bundlers are not supporting this. Next has the same issue and rely on Webpack 5, I think that's also the case for Nuxt according to what I've read online, and maybe other frameworks

@fabis94
Copy link

fabis94 commented Apr 14, 2022

@eric-burel Why not just convert the imports to the format shown in the error message?

@eric-burel
Copy link

@eric-burel Why not just convert the imports to the format shown in the error message?

@benjamn can maybe have more info

I am not sure it's that easy, when your package have a mix of ES modules that can in turn import modules that are either CJS or ESM, you are up to trouble.
Technically you can change the way Apollo expose package via an export map in package.json (that's what I do and I have a setup that works ok, though I need this hot patch on postinstall), but I am not sure if it's the core issue, maybe some Webpack pros could help as well

@eric-burel
Copy link

eric-burel commented May 3, 2022

This breaks with Jest 28 as well, I had to reintroduce @apollo/client and ts-invariant in the build:

// in my jest config
  transformIgnorePatterns: [
    "/node_modules/(?!(@apollo/client|ts-invariant))",
    "^.+\\.module\\.(css|sass|scss)$",
  ],

and also extend my patch to provide a full export map for all internal packages of @apollo/client. Finally, I needed to import "gql" from "graphql-tag" instead of @apollo/client.

Demo: VulcanJS/vulcan-npm#115

@momenthana
Copy link

momenthana commented May 27, 2022

I modified the package.json because I had to use esm only modules(ex: unified, remark ...)

// package.json
...
"type": "module",

a detour by applying it like this in the Jest test

// <rootDir>/lib/test/apollo.cjs
const apollo = require('@apollo/client/main');
const gql = require('graphql-tag');

module.exports = { ...apollo, gql };
// jest.config.js
...
moduleNameMapper: {
  '^@apollo/client$': '<rootDir>/lib/test/apollo',
},

with ts-jest

// jest.config.js
...
preset: 'ts-jest/presets/js-with-ts-esm',
globals: {
  'ts-jest': {
    tsconfig: 'tsconfig.json',
  },
},
// import { ApolloClient, InMemoryCache, NormalizedCacheObject, from, HttpLink } from '@apollo/client';
import { ApolloClient, InMemoryCache, NormalizedCacheObject } from '@apollo/client/core';
import { from } from '@apollo/client/link/core';
import { HttpLink } from '@apollo/client/link/http';

@stephenjason89
Copy link

stephenjason89 commented Jul 12, 2022

Would really appreciate an update on this. Currently using it with nuxt 3

@underblob
Copy link

underblob commented Jul 22, 2022

Node 16 SSR ESM project: we point to the internal index.js in the package. Seems to work.

import { ApolloClient, from, gql } from '@apollo/client/core/index.js';
import { HttpLink } from '@apollo/client/link/http/index.js';
import { InMemoryCache } from '@apollo/client/cache/index.js';
import { createPersistedQueryLink } from '@apollo/client/link/persisted-queries/index.js';

@stephenjason89
Copy link

@underblob Thank you, this seems to work.

@eric-burel
Copy link

eric-burel commented Sep 8, 2022

Here is a gist: https://gist.github.com/eric-burel/543bdc809bcc62208deabdeb004db724 Tested with Apollo 3.5.10 Add node ./fix-apollo.js to postinstall package.json

My patch doesn't work anymore with PNPM symlinking. I am able to provide open source repro, Apollo is really hard to use iin ESM packages...

Edit: ts-invariant 0.10+ changed its ESM module export name. Here is the up to date patch I use to build Next with @apollo/client imported from ESM dependencies: https://github.com/Devographics/Monorepo/blob/5b25381146ab7c5b69df72d9cdb1a0ec154a7864/surveyform/.vn/scripts/fix-apollo.js

@eric-burel
Copy link

eric-burel commented Sep 15, 2022

Node 16 SSR ESM project: we point to the internal index.js in the package. Seems to work.

import { ApolloClient, from, gql } from '@apollo/client/core/index.js';
import { HttpLink } from '@apollo/client/link/http/index.js';
import { InMemoryCache } from '@apollo/client/cache/index.js';
import { createPersistedQueryLink } from '@apollo/client/link/persisted-queries/index.js';

This works but indeed the sign that "@apollo/client/index.js" is not properly interpreted as an ES module in the first place. I have no idea how to force build system to take that into consideration. Maybe renaming to .mjs?

Edit: doesn't work actually, it will still consider relative import as CJS (eg choke on "../versions.js"), I have a (big) repro here: VulcanJS/vulcan-npm#140

@revmischa
Copy link

This works but indeed the sign that "@apollo/client/index.js" is not properly interpreted as an ES module in the first place. I have no idea how to force build system to take that into consideration. Maybe renaming to .mjs?

If using esbuild you can tell it to prefer ESM exports with the mainFields option I believe

@shahyar
Copy link

shahyar commented Sep 21, 2022

It seems that exports map is not an option for Apollo because it would break older applications

What about doing a major release version, which is how is-promise solved this issue?

@RyKilleen
Copy link

Wanted to note a similar but unique challenge here:

I'm using graphql-codegen and the typescript-react-apollo plugin. This generates these imports:

import { gql } from '@apollo/client';
import * as Apollo from '@apollo/client';
// ... codegen types and results

When I try to use any of the exported values from the codegen file in an ESM Typescript project (otherwise working), I receive this error:

import { gql } from '@apollo/client';
         ^^^
SyntaxError: Named export 'gql' not found. The requested module '@apollo/client' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@apollo/client';
const { gql } = pkg;

Changing the imports as suggested makes it work, but I don't have control over the codegen plugin.

@revmischa
Copy link

I just really want tree shaking to work with @apollo/client. No matter what I do the entire thing always gets bundled in my app, and it affects lambda cold starts. It's so bad I am weighing the cost of switching gql clients.

I also am using graphql-codegen and the typescript-react-apollo plugin.

image

@ahnpnl
Copy link

ahnpnl commented Dec 29, 2022

Hi, I ran into this error as well. Will we get this fix soon?

@jerelmiller jerelmiller added the core Feature requests related to core functionality label Apr 6, 2023
@cedeber
Copy link

cedeber commented Apr 20, 2023

Isn't just the issue the fact that Typescript def looks like export * from './core'; instead of export * from './core/index.d.ts'; or something similar. TypeScript moduleResolution set to node16 or nodenext does not support import without file names. It doesn't discover the index magically anymore.

@revmischa
Copy link

TypeScript 5 will magically discover the index file if moduleResolution is set to 'bundler'

@Milutin-P
Copy link

TypeScript 5 will magically discover the index file if moduleResolution is set to 'bundler'

If someone sees this, this is what helped me resolve @apollo/client error in react-native. Thanks @revmischa

{
  "extends": "@tsconfig/react-native/tsconfig.json",
  "compilerOptions": {
    "types": ["react-native"],
    "module": "ES6",
    "moduleResolution": "bundler"
  }
}

@steniowagner
Copy link

@Milutin-P worked like a charm! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Feature requests related to core functionality
Projects
None yet
Development

No branches or pull requests