From 47b798c91237880028e0a5293ccf77fdfc5824e5 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Thu, 23 Jun 2022 11:21:40 -0700 Subject: [PATCH] Fix ESM/CJS and deep imports (#6606) There were a few problems with #6580, which added ESM builds and deep imports to version-4. The `exports` section of `packages/server/package.json` had two mistakes: the `types` lines linked to files in the cjs directory but we only placed them in the esm directory, and the `types` lines are apparently supposed to appear first in each block. However, it turned out that this didn't matter because tsc doesn't even look in the `exports` section unless you set `moduleResolution` to `node16` or `nodenext` in your tsconfig `compilerOptions`, and we weren't! And it turns out doing that setting is a bit of a pain because many packages don't currently work with that flag. So if we published our package with the deep imports only listed in `exports` in package.json, we'd break any TypeScript user who's on the normal `moduleResolution: "node"` and telling them to set `nodenext` would be hard to obey. So since we want to continue to support tsc with `moduleResolution: "node"`, we have to teach tsc about our deep imports using the strategy of "spew a bunch of package.jsons around the package", not even under `src`/`dist`. Fortunately these package.json files can use paths starting with `../` in order to find the actual generated files. So for example, `standalone/package.json` tells tsc how to find `../dist/standalone/index.d.ts`, via fields like `types` and `main`. You might think that now that we have these little package.json files, the `exports` block would be completely redundant. Not so! Node itself does not support deep imports like `@apollo/server/standalone` by looking for a `main` in `standalone/package.json`: when importing from a published package name, it only looks for `main` at the top level of the package. So to support deep imports in Node we need the `exports` block! So this PR leaves both the `exports` block and the tree of `package.json`s in place, to be consumed by different software. We add some tsc use cases to the new smoke test. Our smoke tests verify that consumers with `moduleResolution: "nodenext"` work (or at least, that they will work once https://github.com/ardatan/graphql-tools/pull/4532 is released). However, our own builds don't work that way next because some of our test-only dependencies don't work with that flag. --- .gitignore | 2 + packages/server/.npmignore | 3 ++ packages/server/package.json | 43 ++++++++++--------- .../server/plugin/cacheControl/package.json | 8 ++++ packages/server/plugin/disabled/package.json | 8 ++++ .../plugin/drainHttpServer/package.json | 8 ++++ .../server/plugin/inlineTrace/package.json | 8 ++++ .../plugin/landingPage/default/package.json | 8 ++++ .../graphqlPlayground/package.json | 8 ++++ .../plugin/schemaReporting/package.json | 8 ++++ .../server/plugin/usageReporting/package.json | 8 ++++ .../plugin/{disabled.ts => disabled/index.ts} | 4 +- packages/server/standalone/package.json | 8 ++++ scripts/postcompile.mjs | 3 +- smoke-test/package.json | 1 + smoke-test/prepare.sh | 2 +- smoke-test/smoke-test.cts | 39 +++++++++++++++++ smoke-test/smoke-test.mts | 39 +++++++++++++++++ smoke-test/smoke-test.sh | 5 +++ smoke-test/tsconfig.cjs.json | 12 ++++++ smoke-test/tsconfig.esm.json | 12 ++++++ 21 files changed, 213 insertions(+), 24 deletions(-) create mode 100644 packages/server/plugin/cacheControl/package.json create mode 100644 packages/server/plugin/disabled/package.json create mode 100644 packages/server/plugin/drainHttpServer/package.json create mode 100644 packages/server/plugin/inlineTrace/package.json create mode 100644 packages/server/plugin/landingPage/default/package.json create mode 100644 packages/server/plugin/landingPage/graphqlPlayground/package.json create mode 100644 packages/server/plugin/schemaReporting/package.json create mode 100644 packages/server/plugin/usageReporting/package.json rename packages/server/src/plugin/{disabled.ts => disabled/index.ts} (92%) create mode 100644 packages/server/standalone/package.json create mode 100644 smoke-test/smoke-test.cts create mode 100644 smoke-test/smoke-test.mts create mode 100644 smoke-test/tsconfig.cjs.json create mode 100644 smoke-test/tsconfig.esm.json diff --git a/.gitignore b/.gitignore index 0fd96213c03..092135e4ad5 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,5 @@ node_modules/ # Volta binaries (when using direnv/.envrc) .volta + +smoke-test/generated diff --git a/packages/server/.npmignore b/packages/server/.npmignore index 7514d963d20..a4ce211d55e 100644 --- a/packages/server/.npmignore +++ b/packages/server/.npmignore @@ -1,4 +1,7 @@ * +# Include the package.json files that give us deep imports. +!**/package.json +node_modules/**/* !src/**/* src/**/__tests__/** !dist/**/* diff --git a/packages/server/package.json b/packages/server/package.json index a2d1782fbe9..317e565f9ab 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -3,56 +3,59 @@ "version": "3.6.7", "description": "Core engine for Apollo GraphQL server", "type": "module", + "main": "dist/cjs/index.js", + "module": "dist/esm/index.js", + "types": "dist/esm/index.d.ts", "exports": { ".": { + "types": "./dist/esm/index.d.ts", "import": "./dist/esm/index.js", - "require": "./dist/cjs/index.js", - "types": "./dist/cjs/index.d.ts" + "require": "./dist/cjs/index.js" }, "./standalone": { + "types": "./dist/esm/standalone/index.d.ts", "import": "./dist/esm/standalone/index.js", - "require": "./dist/cjs/standalone/index.js", - "types": "./dist/cjs/standalone/index.d.ts" + "require": "./dist/cjs/standalone/index.js" }, "./plugin/cacheControl": { + "types": "./dist/esm/plugin/cacheControl/index.d.ts", "import": "./dist/esm/plugin/cacheControl/index.js", - "require": "./dist/cjs/plugin/cacheControl/index.js", - "types": "./dist/cjs/plugin/cacheControl/index.d.ts" + "require": "./dist/cjs/plugin/cacheControl/index.js" }, "./plugin/disabled": { + "types": "./dist/esm/plugin/disabled.d.ts", "import": "./dist/esm/plugin/disabled.js", - "require": "./dist/cjs/plugin/disabled.js", - "types": "./dist/cjs/plugin/disabled.d.ts" + "require": "./dist/cjs/plugin/disabled.js" }, "./plugin/drainHttpServer": { + "types": "./dist/esm/plugin/drainHttpServer/index.d.ts", "import": "./dist/esm/plugin/drainHttpServer/index.js", - "require": "./dist/cjs/plugin/drainHttpServer/index.js", - "types": "./dist/cjs/plugin/drainHttpServer/index.d.ts" + "require": "./dist/cjs/plugin/drainHttpServer/index.js" }, "./plugin/inlineTrace": { + "types": "./dist/esm/plugin/inlineTrace/index.d.ts", "import": "./dist/esm/plugin/inlineTrace/index.js", - "require": "./dist/cjs/plugin/inlineTrace/index.js", - "types": "./dist/cjs/plugin/inlineTrace/index.d.ts" + "require": "./dist/cjs/plugin/inlineTrace/index.js" }, "./plugin/landingPage/default": { + "types": "./dist/esm/plugin/landingPage/default/index.d.ts", "import": "./dist/esm/plugin/landingPage/default/index.js", - "require": "./dist/cjs/plugin/landingPage/default/index.js", - "types": "./dist/cjs/plugin/landingPage/default/index.d.ts" + "require": "./dist/cjs/plugin/landingPage/default/index.js" }, "./plugin/landingPage/graphqlPlayground": { + "types": "./dist/esm/plugin/landingPage/graphqlPlayground/index.d.ts", "import": "./dist/esm/plugin/landingPage/graphqlPlayground/index.js", - "require": "./dist/cjs/plugin/landingPage/graphqlPlayground/index.js", - "types": "./dist/cjs/plugin/landingPage/graphqlPlayground/index.d.ts" + "require": "./dist/cjs/plugin/landingPage/graphqlPlayground/index.js" }, "./plugin/schemaReporting": { + "types": "./dist/esm/plugin/schemaReporting/index.d.ts", "import": "./dist/esm/plugin/schemaReporting/index.js", - "require": "./dist/cjs/plugin/schemaReporting/index.js", - "types": "./dist/cjs/plugin/schemaReporting/index.d.ts" + "require": "./dist/cjs/plugin/schemaReporting/index.js" }, "./plugin/usageReporting": { + "types": "./dist/esm/plugin/usageReporting/index.d.ts", "import": "./dist/esm/plugin/usageReporting/index.js", - "require": "./dist/cjs/plugin/usageReporting/index.js", - "types": "./dist/cjs/plugin/usageReporting/index.d.ts" + "require": "./dist/cjs/plugin/usageReporting/index.js" } }, "repository": { diff --git a/packages/server/plugin/cacheControl/package.json b/packages/server/plugin/cacheControl/package.json new file mode 100644 index 00000000000..284d6cbaa5a --- /dev/null +++ b/packages/server/plugin/cacheControl/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/cacheControl", + "type": "module", + "main": "../dist/cjs/plugin/cacheControl/index.js", + "module": "../dist/esm/plugin/cacheControl/index.js", + "types": "../dist/esm/plugin/cacheControl/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/plugin/disabled/package.json b/packages/server/plugin/disabled/package.json new file mode 100644 index 00000000000..c36a4823904 --- /dev/null +++ b/packages/server/plugin/disabled/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/disabled", + "type": "module", + "main": "../dist/cjs/plugin/disabled/index.js", + "module": "../dist/esm/plugin/disabled/index.js", + "types": "../dist/esm/plugin/disabled/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/plugin/drainHttpServer/package.json b/packages/server/plugin/drainHttpServer/package.json new file mode 100644 index 00000000000..0d5fda142d0 --- /dev/null +++ b/packages/server/plugin/drainHttpServer/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/drainHttpServer", + "type": "module", + "main": "../dist/cjs/plugin/drainHttpServer/index.js", + "module": "../dist/esm/plugin/drainHttpServer/index.js", + "types": "../dist/esm/plugin/drainHttpServer/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/plugin/inlineTrace/package.json b/packages/server/plugin/inlineTrace/package.json new file mode 100644 index 00000000000..ad86d25b710 --- /dev/null +++ b/packages/server/plugin/inlineTrace/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/inlineTrace", + "type": "module", + "main": "../dist/cjs/plugin/inlineTrace/index.js", + "module": "../dist/esm/plugin/inlineTrace/index.js", + "types": "../dist/esm/plugin/inlineTrace/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/plugin/landingPage/default/package.json b/packages/server/plugin/landingPage/default/package.json new file mode 100644 index 00000000000..245258ad0bf --- /dev/null +++ b/packages/server/plugin/landingPage/default/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/landingPage/default", + "type": "module", + "main": "../dist/cjs/plugin/landingPage/default/index.js", + "module": "../dist/esm/plugin/landingPage/default/index.js", + "types": "../dist/esm/plugin/landingPage/default/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/plugin/landingPage/graphqlPlayground/package.json b/packages/server/plugin/landingPage/graphqlPlayground/package.json new file mode 100644 index 00000000000..fe508f66b8e --- /dev/null +++ b/packages/server/plugin/landingPage/graphqlPlayground/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/landingPage/graphqlPlayground", + "type": "module", + "main": "../dist/cjs/plugin/landingPage/graphqlPlayground/index.js", + "module": "../dist/esm/plugin/landingPage/graphqlPlayground/index.js", + "types": "../dist/esm/plugin/landingPage/graphqlPlayground/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/plugin/schemaReporting/package.json b/packages/server/plugin/schemaReporting/package.json new file mode 100644 index 00000000000..b1986429d63 --- /dev/null +++ b/packages/server/plugin/schemaReporting/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/schemaReporting", + "type": "module", + "main": "../dist/cjs/plugin/schemaReporting/index.js", + "module": "../dist/esm/plugin/schemaReporting/index.js", + "types": "../dist/esm/plugin/schemaReporting/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/plugin/usageReporting/package.json b/packages/server/plugin/usageReporting/package.json new file mode 100644 index 00000000000..0abfb64ddf3 --- /dev/null +++ b/packages/server/plugin/usageReporting/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/plugin/usageReporting", + "type": "module", + "main": "../dist/cjs/plugin/usageReporting/index.js", + "module": "../dist/esm/plugin/usageReporting/index.js", + "types": "../dist/esm/plugin/usageReporting/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/packages/server/src/plugin/disabled.ts b/packages/server/src/plugin/disabled/index.ts similarity index 92% rename from packages/server/src/plugin/disabled.ts rename to packages/server/src/plugin/disabled/index.ts index a9bd0d3ea92..ddc1e8212b1 100644 --- a/packages/server/src/plugin/disabled.ts +++ b/packages/server/src/plugin/disabled/index.ts @@ -1,10 +1,10 @@ // TODO(AS4): Document this file // TODO(AS4): Decide where it is imported from. -import type { BaseContext, ApolloServerPlugin } from '..'; +import type { BaseContext, ApolloServerPlugin } from '../..'; import type { InternalApolloServerPlugin, InternalPluginId, -} from '../internalPlugin'; +} from '../../internalPlugin'; function disabledPlugin( id: InternalPluginId, diff --git a/packages/server/standalone/package.json b/packages/server/standalone/package.json new file mode 100644 index 00000000000..258e4369b45 --- /dev/null +++ b/packages/server/standalone/package.json @@ -0,0 +1,8 @@ +{ + "name": "@apollo/server/standalone", + "type": "module", + "main": "../dist/cjs/standalone/index.js", + "module": "../dist/esm/standalone/index.js", + "types": "../dist/esm/standalone/index.d.ts", + "sideEffects": false +} \ No newline at end of file diff --git a/scripts/postcompile.mjs b/scripts/postcompile.mjs index 1d6a39e80e3..07c9e821cd2 100644 --- a/scripts/postcompile.mjs +++ b/scripts/postcompile.mjs @@ -11,6 +11,7 @@ import path from 'path'; import { writeFileSync } from 'fs'; import rimraf from 'rimraf'; +// Tell Node what kinds of files the ".js" files in these subdirectories are. writeFileSync( path.join('packages', 'server', 'dist', 'esm', 'package.json'), JSON.stringify({ type: 'module' }), @@ -20,5 +21,5 @@ writeFileSync( JSON.stringify({ type: 'commonjs' }), ); -// Remove CJS .d.ts files. +// Remove CJS .d.ts files: we don't need two copies! rimraf.sync('packages/server/dist/cjs/**/*.d.ts'); diff --git a/smoke-test/package.json b/smoke-test/package.json index e948d9271aa..a8e11201d73 100644 --- a/smoke-test/package.json +++ b/smoke-test/package.json @@ -7,6 +7,7 @@ "@rollup/plugin-commonjs": "22.0.0", "@rollup/plugin-json": "4.1.0", "@rollup/plugin-node-resolve": "13.3.0", + "@types/make-fetch-happen": "9.0.2", "rollup": "2.75.7" } } diff --git a/smoke-test/prepare.sh b/smoke-test/prepare.sh index 874c6f118e9..153c90c2447 100755 --- a/smoke-test/prepare.sh +++ b/smoke-test/prepare.sh @@ -10,7 +10,7 @@ npm pack --pack-destination="$TARBALL_DIR" --workspaces=true # Install node_modules in the smoke-test directory cd smoke-test -rm -rf node_modules package-lock.json +rm -rf node_modules package-lock.json generated # First install normal dependencies npm i # Now install the tarballs we made (but don't write their paths to package.json) diff --git a/smoke-test/smoke-test.cts b/smoke-test/smoke-test.cts new file mode 100644 index 00000000000..eca02014471 --- /dev/null +++ b/smoke-test/smoke-test.cts @@ -0,0 +1,39 @@ +import { ApolloServer } from '@apollo/server'; +import { startStandaloneServer } from '@apollo/server/standalone'; +import fetch from 'make-fetch-happen'; +import assert from 'assert'; + +async function smokeTest() { + const s = new ApolloServer({ + typeDefs: 'type Query {hello:String}', + resolvers: { + Query: { + hello() { + return 'world'; + }, + }, + }, + }); + const { url } = await startStandaloneServer(s, { listen: { port: 0 } }); + + const response = await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ query: '{hello}' }), + }); + const body = await response.json(); + + assert.strictEqual(body.data.hello, 'world'); + + await s.stop(); +} + +smokeTest() + .then(() => { + console.log('TS-CJS smoke test passed!'); + process.exit(0); + }) + .catch((e) => { + console.error(e); + process.exit(1); + }); diff --git a/smoke-test/smoke-test.mts b/smoke-test/smoke-test.mts new file mode 100644 index 00000000000..49beb3bcc83 --- /dev/null +++ b/smoke-test/smoke-test.mts @@ -0,0 +1,39 @@ +import { ApolloServer } from '@apollo/server'; +import { startStandaloneServer } from '@apollo/server/standalone'; +import fetch from 'make-fetch-happen'; +import assert from 'assert'; + +async function smokeTest() { + const s = new ApolloServer({ + typeDefs: 'type Query {hello:String}', + resolvers: { + Query: { + hello() { + return 'world'; + }, + }, + }, + }); + const { url } = await startStandaloneServer(s, { listen: { port: 0 } }); + + const response = await fetch(url, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ query: '{hello}' }), + }); + const body = await response.json(); + + assert.strictEqual(body.data.hello, 'world'); + + await s.stop(); +} + +smokeTest() + .then(() => { + console.log('TS-ESM smoke test passed!'); + process.exit(0); + }) + .catch((e) => { + console.error(e); + process.exit(1); + }); diff --git a/smoke-test/smoke-test.sh b/smoke-test/smoke-test.sh index bf743495a29..c084965d2cc 100755 --- a/smoke-test/smoke-test.sh +++ b/smoke-test/smoke-test.sh @@ -22,3 +22,8 @@ npx rollup smoke-test-no-express.mjs --config rollup.config.js --silent --file " grep 'function createApplication' "$ROLLUP_OUT_DIR"/bundle.mjs # ... and that the one that doesn't, doesn't. ! grep 'function createApplication' "$ROLLUP_OUT_DIR"/bundle-no-express.mjs + +# Ensure basic TypeScript builds work. +tsc --build tsconfig.{esm,cjs}.json +node generated/tsc/smoke-test.cjs +node generated/tsc/smoke-test.mjs diff --git a/smoke-test/tsconfig.cjs.json b/smoke-test/tsconfig.cjs.json new file mode 100644 index 00000000000..79d3af45df7 --- /dev/null +++ b/smoke-test/tsconfig.cjs.json @@ -0,0 +1,12 @@ +{ + "files": [ + "smoke-test.cts", + ], + "compilerOptions": { + "outDir": "generated/tsc", + "module": "commonjs", + "moduleResolution": "node", + "esModuleInterop": true, + "strict": true, + }, +} diff --git a/smoke-test/tsconfig.esm.json b/smoke-test/tsconfig.esm.json new file mode 100644 index 00000000000..ae06a93c6b0 --- /dev/null +++ b/smoke-test/tsconfig.esm.json @@ -0,0 +1,12 @@ +{ + "files": [ + "smoke-test.mts", + ], + "compilerOptions": { + "outDir": "generated/tsc", + "module": "esnext", + "moduleResolution": "node", + "esModuleInterop": true, + "strict": true, + }, +}