Skip to content

Commit

Permalink
use conditional imports instead of environment for instanceOf checks
Browse files Browse the repository at this point in the history
  • Loading branch information
yaacovCR committed Oct 9, 2024
1 parent 8b86cd2 commit 930619a
Show file tree
Hide file tree
Showing 17 changed files with 302 additions and 130 deletions.
10 changes: 10 additions & 0 deletions integrationTests/node/index.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { readFileSync } = require('fs');
const {
experimentalExecuteIncrementally,
graphqlSync,
isSchema,
parse,
} = require('graphql');
const { buildSchema } = require('graphql/utilities');
Expand Down Expand Up @@ -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);
10 changes: 10 additions & 0 deletions integrationTests/node/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { readFileSync } from 'fs';
import {
experimentalExecuteIncrementally,
graphqlSync,
isSchema,
parse,
} from 'graphql-esm';
import { buildSchema } from 'graphql-esm/utilities';
Expand Down Expand Up @@ -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);
21 changes: 21 additions & 0 deletions integrationTests/node/instanceOfForDevelopment.cjs
Original file line number Diff line number Diff line change
@@ -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,
),
);
22 changes: 22 additions & 0 deletions integrationTests/node/instanceOfForDevelopment.mjs
Original file line number Diff line number Diff line change
@@ -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,
),
);
12 changes: 4 additions & 8 deletions integrationTests/node/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
}
}
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
43 changes: 42 additions & 1 deletion resources/build-deno.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) ||
Expand Down Expand Up @@ -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);
}
}
}
43 changes: 41 additions & 2 deletions resources/build-npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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' });
}
Expand All @@ -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;
Expand All @@ -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],
});
Expand Down
5 changes: 5 additions & 0 deletions resources/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 };
Expand Down
62 changes: 1 addition & 61 deletions src/jsutils/__tests__/instanceOf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}
Expand All @@ -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,
);
});
});
Loading

0 comments on commit 930619a

Please sign in to comment.