use conditional imports instead of environment for instanceOf checks

graphql · Oct 9, 2024 · 930619a · 930619a
1 parent 8b86cd2
commit 930619a
Show file tree

Hide file tree

Showing 17 changed files with 302 additions and 130 deletions.
diff --git a/integrationTests/node/index.cjs b/integrationTests/node/index.cjs
@@ -4,6 +4,7 @@ const { readFileSync } = require('fs');
 const {
   experimentalExecuteIncrementally,
   graphqlSync,
+  isSchema,
   parse,
 } = require('graphql');
 const { buildSchema } = require('graphql/utilities');
@@ -52,3 +53,12 @@ result = experimentalExecuteIncrementally({
 
 assert(result.errors?.[0] !== undefined);
 assert(!result.errors[0].message.includes('is not defined'));
+
+// test instanceOf without development condition
+class GraphQLSchema {
+  get [Symbol.toStringTag]() {
+    return 'GraphQLSchema';
+  }
+}
+const notSchema = new GraphQLSchema();
+assert(isSchema(notSchema) === false);
diff --git a/integrationTests/node/index.mjs b/integrationTests/node/index.mjs
@@ -5,6 +5,7 @@ import { readFileSync } from 'fs';
 import {
   experimentalExecuteIncrementally,
   graphqlSync,
+  isSchema,
   parse,
 } from 'graphql-esm';
 import { buildSchema } from 'graphql-esm/utilities';
@@ -53,3 +54,12 @@ result = experimentalExecuteIncrementally({
 
 assert(result.errors?.[0] !== undefined);
 assert(!result.errors[0].message.includes('is not defined'));
+
+// test instanceOf without development condition
+class GraphQLSchema {
+  get [Symbol.toStringTag]() {
+    return 'Source';
+  }
+}
+const notSchema = new GraphQLSchema();
+assert(isSchema(notSchema) === false);
diff --git a/integrationTests/node/instanceOfForDevelopment.cjs b/integrationTests/node/instanceOfForDevelopment.cjs
@@ -0,0 +1,21 @@
+const assert = require('assert');
+
+const { isSchema } = require('graphql');
+
+class GraphQLSchema {
+  get [Symbol.toStringTag]() {
+    return 'GraphQLSchema';
+  }
+}
+const notSchema = new GraphQLSchema();
+let error;
+try {
+  isSchema(notSchema);
+} catch (_error) {
+  error = _error;
+}
+assert(
+  String(error?.message).match(
+    /^Cannot use GraphQLSchema "{}" from another module or realm./m,
+  ),
+);
diff --git a/integrationTests/node/instanceOfForDevelopment.mjs b/integrationTests/node/instanceOfForDevelopment.mjs
@@ -0,0 +1,22 @@
+/* eslint-disable simple-import-sort/imports */
+import assert from 'assert';
+
+import { isSchema } from 'graphql-esm';
+
+class GraphQLSchema {
+  get [Symbol.toStringTag]() {
+    return 'GraphQLSchema';
+  }
+}
+const notSchema = new GraphQLSchema();
+let error;
+try {
+  isSchema(notSchema);
+} catch (_error) {
+  error = _error;
+}
+assert(
+  String(error?.message).match(
+    /^Cannot use GraphQLSchema "{}" from another module or realm./m,
+  ),
+);
diff --git a/integrationTests/node/test.js b/integrationTests/node/test.js
@@ -14,13 +14,9 @@ const nodeVersions = graphqlPackageJSON.engines.node
 for (const version of nodeVersions) {
   console.log(`Testing on node@${version} ...`);
 
-  childProcess.execSync(
-    `docker run --rm --volume "$PWD":/usr/src/app -w /usr/src/app node:${version}-slim node ./index.cjs`,
-    { stdio: 'inherit' },
-  );
+  for (const test of ['index', 'instanceOfForDevelopment']) {
+    childProcess.execSync(`node ./${test}.cjs`, { stdio: 'inherit' });
 
-  childProcess.execSync(
-    `docker run --rm --volume "$PWD":/usr/src/app -w /usr/src/app node:${version}-slim node ./index.mjs`,
-    { stdio: 'inherit' },
-  );
+    childProcess.execSync(`node ./${test}.mjs`, { stdio: 'inherit' });
+  }
 }
diff --git a/package.json b/package.json
@@ -28,6 +28,12 @@
   "engines": {
     "node": "^16.19.0 || ^18.14.0 || >=19.7.0"
   },
+  "imports": {
+    "#instanceOf": {
+      "development": "./src/jsutils/instanceOfForDevelopment.ts",
+      "default": "./src/jsutils/instanceOf.ts"
+    }
+  },
   "scripts": {
     "preversion": "bash -c '. ./resources/checkgit.sh && npm ci --ignore-scripts'",
     "version": "node --loader ts-node/esm resources/gen-version.ts && npm test && git add src/version.ts",

diff --git a/resources/build-deno.ts b/resources/build-deno.ts
@@ -5,8 +5,10 @@ import ts from 'typescript';
 
 import { changeExtensionInImportPaths } from './change-extension-in-import-paths.js';
 import { inlineInvariant } from './inline-invariant.js';
+import type { ImportsMap } from './utils.js';
 import {
   prettify,
+  readPackageJSON,
   readTSConfig,
   showDirStats,
   writeGeneratedFile,
@@ -15,7 +17,10 @@ import {
 fs.rmSync('./denoDist', { recursive: true, force: true });
 fs.mkdirSync('./denoDist');
 
-const tsProgram = ts.createProgram(['src/index.ts'], readTSConfig());
+const tsProgram = ts.createProgram(
+  ['src/index.ts', 'src/jsutils/instanceOf.ts'],
+  readTSConfig(),
+);
 for (const sourceFile of tsProgram.getSourceFiles()) {
   if (
     tsProgram.isSourceFileFromExternalLibrary(sourceFile) ||
@@ -45,4 +50,40 @@ for (const sourceFile of tsProgram.getSourceFiles()) {
 fs.copyFileSync('./LICENSE', './denoDist/LICENSE');
 fs.copyFileSync('./README.md', './denoDist/README.md');
 
+const imports = getImports();
+const importsJsonPath = `./denoDist/imports.json`;
+const prettified = await prettify(importsJsonPath, JSON.stringify(imports));
+writeGeneratedFile(importsJsonPath, prettified);
+
 showDirStats('./denoDist');
+
+function getImports(): ImportsMap {
+  const packageJSON = readPackageJSON();
+  const newImports: ImportsMap = {};
+  for (const [key, value] of Object.entries(packageJSON.imports)) {
+    if (typeof value === 'string') {
+      newImports[key] = updateImportPath(value, '.ts');
+      continue;
+    }
+    const denoValue = findDenoValue(value);
+    if (denoValue !== undefined) {
+      newImports[key] = updateImportPath(denoValue, '.ts');
+    }
+  }
+  return newImports;
+}
+
+function updateImportPath(value: string, extension: string) {
+  return value.replace(/\/src\//g, '/').replace(/\.ts$/, extension);
+}
+
+function findDenoValue(importsMap: ImportsMap): string | undefined {
+  for (const [key, value] of Object.entries(importsMap)) {
+    if (key === 'deno' || key === 'default') {
+      if (typeof value === 'string') {
+        return value;
+      }
+      return findDenoValue(value);
+    }
+  }
+}
diff --git a/resources/build-npm.ts b/resources/build-npm.ts
@@ -6,6 +6,7 @@ import ts from 'typescript';
 
 import { changeExtensionInImportPaths } from './change-extension-in-import-paths.js';
 import { inlineInvariant } from './inline-invariant.js';
+import type { ImportsMap } from './utils.js';
 import {
   prettify,
   readPackageJSON,
@@ -102,12 +103,23 @@ async function buildPackage(outDir: string, isESMOnly: boolean): Promise<void> {
     packageJSON.exports['./*.js'] = './*.js';
     packageJSON.exports['./*'] = './*.js';
 
+    packageJSON.imports = mapImports(packageJSON.imports, (value: string) =>
+      updateImportPath(value, '.js'),
+    );
+
+    packageJSON.type = 'module';
     packageJSON.publishConfig.tag += '-esm';
     packageJSON.version += '+esm';
   } else {
-    delete packageJSON.type;
+    packageJSON.type = 'commonjs';
     packageJSON.main = 'index';
     packageJSON.module = 'index.mjs';
+
+    packageJSON.imports = mapImports(packageJSON.imports, (value: string) => ({
+      import: updateImportPath(value, '.mjs'),
+      default: updateImportPath(value, '.js'),
+    }));
+
     emitTSFiles({ outDir, module: 'commonjs', extension: '.js' });
     emitTSFiles({ outDir, module: 'es2020', extension: '.mjs' });
   }
@@ -121,6 +133,25 @@ async function buildPackage(outDir: string, isESMOnly: boolean): Promise<void> {
   writeGeneratedFile(packageJsonPath, prettified);
 }
 
+function updateImportPath(value: string, extension: string) {
+  return value.replace(/\/src\//g, '/').replace(/\.ts$/, extension);
+}
+
+function mapImports(
+  imports: ImportsMap,
+  replacer: (value: string) => string | ImportsMap,
+): ImportsMap {
+  const newImports: ImportsMap = {};
+  for (const [key, value] of Object.entries(imports)) {
+    if (typeof value === 'string') {
+      newImports[key] = replacer(value);
+      continue;
+    }
+    newImports[key] = mapImports(value, replacer);
+  }
+  return newImports;
+}
+
 // Based on https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API#getting-the-dts-from-a-javascript-file
 function emitTSFiles(options: {
   outDir: string;
@@ -143,7 +174,15 @@ function emitTSFiles(options: {
   tsHost.writeFile = (filepath, body) =>
     writeGeneratedFile(filepath.replace(/.js$/, extension), body);
 
-  const tsProgram = ts.createProgram(['src/index.ts'], tsOptions, tsHost);
+  const tsProgram = ts.createProgram(
+    [
+      'src/index.ts',
+      'src/jsutils/instanceOf.ts',
+      'src/jsutils/instanceOfForDevelopment.ts',
+    ],
+    tsOptions,
+    tsHost,
+  );
   const tsResult = tsProgram.emit(undefined, undefined, undefined, undefined, {
     after: [changeExtensionInImportPaths({ extension }), inlineInvariant],
   });

diff --git a/resources/utils.ts b/resources/utils.ts
@@ -227,6 +227,10 @@ export function writeGeneratedFile(filepath: string, body: string): void {
   fs.writeFileSync(filepath, body);
 }
 
+export interface ImportsMap {
+  [path: string]: string | ImportsMap;
+}
+
 interface PackageJSON {
   description: string;
   version: string;
@@ -235,6 +239,7 @@ interface PackageJSON {
   scripts?: { [name: string]: string };
   type?: string;
   exports: { [path: string]: string };
+  imports: ImportsMap;
   types?: string;
   typesVersions: { [ranges: string]: { [path: string]: Array<string> } };
   devDependencies?: { [name: string]: string };

diff --git a/src/jsutils/__tests__/instanceOf-test.ts b/src/jsutils/__tests__/instanceOf-test.ts
@@ -7,6 +7,7 @@ describe('instanceOf', () => {
   it('do not throw on values without prototype', () => {
     class Foo {
       get [Symbol.toStringTag]() {
+        /* c8 ignore next 2 */
         return 'Foo';
       }
     }
@@ -15,65 +16,4 @@ describe('instanceOf', () => {
     expect(instanceOf(null, Foo)).to.equal(false);
     expect(instanceOf(Object.create(null), Foo)).to.equal(false);
   });
-
-  it('detect name clashes with older versions of this lib', () => {
-    function oldVersion() {
-      class Foo {}
-      return Foo;
-    }
-
-    function newVersion() {
-      class Foo {
-        get [Symbol.toStringTag]() {
-          return 'Foo';
-        }
-      }
-      return Foo;
-    }
-
-    const NewClass = newVersion();
-    const OldClass = oldVersion();
-    expect(instanceOf(new NewClass(), NewClass)).to.equal(true);
-    expect(() => instanceOf(new OldClass(), NewClass)).to.throw();
-  });
-
-  it('allows instances to have share the same constructor name', () => {
-    function getMinifiedClass(tag: string) {
-      class SomeNameAfterMinification {
-        get [Symbol.toStringTag]() {
-          return tag;
-        }
-      }
-      return SomeNameAfterMinification;
-    }
-
-    const Foo = getMinifiedClass('Foo');
-    const Bar = getMinifiedClass('Bar');
-    expect(instanceOf(new Foo(), Bar)).to.equal(false);
-    expect(instanceOf(new Bar(), Foo)).to.equal(false);
-
-    const DuplicateOfFoo = getMinifiedClass('Foo');
-    expect(() => instanceOf(new DuplicateOfFoo(), Foo)).to.throw();
-    expect(() => instanceOf(new Foo(), DuplicateOfFoo)).to.throw();
-  });
-
-  it('fails with descriptive error message', () => {
-    function getFoo() {
-      class Foo {
-        get [Symbol.toStringTag]() {
-          return 'Foo';
-        }
-      }
-      return Foo;
-    }
-    const Foo1 = getFoo();
-    const Foo2 = getFoo();
-
-    expect(() => instanceOf(new Foo1(), Foo2)).to.throw(
-      /^Cannot use Foo "{}" from another module or realm./m,
-    );
-    expect(() => instanceOf(new Foo2(), Foo1)).to.throw(
-      /^Cannot use Foo "{}" from another module or realm./m,
-    );
-  });
 });