Skip to content

Commit

Permalink
Fix ESM/CJS and deep imports (#6606)
Browse files Browse the repository at this point in the history
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 ardatan/graphql-tools#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.
  • Loading branch information
glasser committed Jun 23, 2022
1 parent f9799de commit 47b798c
Show file tree
Hide file tree
Showing 21 changed files with 213 additions and 24 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Expand Up @@ -32,3 +32,5 @@ node_modules/

# Volta binaries (when using direnv/.envrc)
.volta

smoke-test/generated
3 changes: 3 additions & 0 deletions 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/**/*
Expand Down
43 changes: 23 additions & 20 deletions packages/server/package.json
Expand Up @@ -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": {
Expand Down
8 changes: 8 additions & 0 deletions 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
}
8 changes: 8 additions & 0 deletions 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
}
8 changes: 8 additions & 0 deletions 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
}
8 changes: 8 additions & 0 deletions 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
}
8 changes: 8 additions & 0 deletions 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
}
@@ -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
}
8 changes: 8 additions & 0 deletions 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
}
8 changes: 8 additions & 0 deletions 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
}
@@ -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<TContext extends BaseContext>(
id: InternalPluginId,
Expand Down
8 changes: 8 additions & 0 deletions 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
}
3 changes: 2 additions & 1 deletion scripts/postcompile.mjs
Expand Up @@ -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' }),
Expand All @@ -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');
1 change: 1 addition & 0 deletions smoke-test/package.json
Expand Up @@ -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"
}
}
2 changes: 1 addition & 1 deletion smoke-test/prepare.sh
Expand Up @@ -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)
Expand Down
39 changes: 39 additions & 0 deletions 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);
});
39 changes: 39 additions & 0 deletions 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);
});
5 changes: 5 additions & 0 deletions smoke-test/smoke-test.sh
Expand Up @@ -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
12 changes: 12 additions & 0 deletions 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,
},
}
12 changes: 12 additions & 0 deletions 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,
},
}

0 comments on commit 47b798c

Please sign in to comment.