apollographql / apollo-feature-requests

🧑‍🚀 Apollo Client Feature Requests | (no 🐛 please).
Other
130 stars 7 forks source link

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

Open brainkim opened 3 years ago

brainkim commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

eric-burel commented 2 years ago

@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 commented 2 years ago

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: https://github.com/VulcanJS/vulcan-npm/pull/115

momenthana commented 2 years ago

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 commented 2 years ago

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

underblob commented 2 years ago

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 commented 2 years ago

@underblob Thank you, this seems to work.

eric-burel commented 2 years ago

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 commented 2 years ago

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: https://github.com/VulcanJS/vulcan-npm/pull/140

revmischa commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

Wanted to note a similar but unique challenge here:

I'm usinggraphql-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 commented 1 year ago

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 commented 1 year ago

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

cedeber commented 1 year ago

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 commented 1 year ago

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

Milutin-P commented 1 year ago

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 commented 11 months ago

@Milutin-P worked like a charm! Thanks!