Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Commit

Permalink
Use an instanceOf that warns instead of throwing on duplicate modules
Browse files Browse the repository at this point in the history
  • Loading branch information
Jackson Kearl committed Aug 20, 2019
1 parent 3fc10e2 commit 9ae0995
Show file tree
Hide file tree
Showing 8 changed files with 216 additions and 71 deletions.
77 changes: 77 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@
"node": ">= 6.x"
},
"scripts": {
"test": "npm run prettier:check && npm run lint && npm run check && npm run testonly",
"test": "npm run prettier:check && npm run lint && npm run check && npm run testonly && npm run test:double-dependency",
"test:ci": "npm run prettier:check && npm run lint -- --no-cache && npm run check && npm run testonly:cover && npm run build",
"testonly": "mocha -r ts-node/register --full-trace src/**/__tests__/**/*-test.js",
"testonly": "mocha -r ts-node/register --full-trace src/**/__tests__/**/*-test.js src/**/__tests__/**/*-test.ts",
"testonly:cover": "nyc npm run testonly",
"test:double-dependency": "npm run build && mocha -r ts-node/register --full-trace src/__tests__/double-dependency.test.ts; rm -rf ./dist1",
"lint": "eslint --cache --report-unused-disable-directives src resources",
"benchmark": "node --noconcurrent_sweeping --expose-gc --predictable ./resources/benchmark.js",
"prettier": "prettier --ignore-path .gitignore --write --list-different \"**/*.{js,md,json,yml}\"",
Expand All @@ -54,6 +55,10 @@
"@babel/polyfill": "7.4.4",
"@babel/preset-env": "7.5.5",
"@babel/register": "7.5.5",
"@types/chai": "^4.2.0",
"@types/mocha": "^5.2.7",
"@types/node": "^12.7.2",
"@types/shelljs": "^0.8.5",
"babel-eslint": "10.0.2",
"chai": "4.2.0",
"eslint": "5.16.0",
Expand All @@ -62,6 +67,7 @@
"mocha": "6.2.0",
"nyc": "14.1.1",
"prettier": "1.18.2",
"shelljs": "0.8.3",
"ts-node": "^8.3.0",
"typescript": "^3.5.3"
}
Expand Down
35 changes: 35 additions & 0 deletions src/__tests__/double-dependency.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import * as shelljs from 'shelljs';
import { join } from 'path';

import { expect } from 'chai';
import { describe, it } from 'mocha';

describe('graphql-js module', () => {
it('allows interop between two versions of iteslf', () => {
const warn = console.warn;
const warnedWith = [];
console.warn = message => warnedWith.push(message);
const root = shelljs.pwd().stdout;

const g1Path = 'dist';
const g2Path = 'dist1';

shelljs.cp('-r', g1Path, g2Path);

const g1 = require(join(root, g1Path));
const g2 = require(join(root, g2Path));

const obj = new g1.GraphQLObjectType({
name: 'Dog',
fields: { id: { type: g1.GraphQLString } },
});

expect(g1.isObjectType(obj)).to.be.true;
expect(g2.isObjectType(obj)).to.be.true;
expect(warnedWith[0]).to.include(
'Using GraphQLObjectType "Dog" from another module or realm.',
);
expect(warnedWith[1]).to.be.undefined;
console.warn = warn;
}).timeout(5000);
});
24 changes: 0 additions & 24 deletions src/jsutils/__tests__/instanceOf-test.js

This file was deleted.

38 changes: 38 additions & 0 deletions src/jsutils/__tests__/instanceOf-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// @flow strict

import { expect } from 'chai';
import { describe, it } from 'mocha';

import instanceOf from '../instanceOf';

describe('instanceOf', () => {
it('fails with descriptive error message', () => {
let calledWith = [];
const warn = console.warn;
console.warn = message => calledWith.push(message);

function getFoo() {
class Foo {}
return Foo;
}

function getBar() {
return Bar;
}

const Foo1 = getFoo();
const Foo2 = getFoo();

class Bar {}
const bar = new Bar();

expect(calledWith[0]).to.be.undefined;
expect(instanceOf(new Foo1(), Foo2)).to.be.true;
expect(calledWith[0]).to.not.be.undefined;
expect(instanceOf(new Foo2(), Foo1)).to.be.true;
expect(calledWith[1]).to.be.undefined; // only warn once

expect(instanceOf(bar, Foo1)).to.be.false;
console.warn = warn;
});
});
45 changes: 0 additions & 45 deletions src/jsutils/instanceOf.js

This file was deleted.

10 changes: 10 additions & 0 deletions src/jsutils/instanceOf.js.flow
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// @flow strict

/**
* A replacement for instanceof which includes an error warning when multi-realm
* constructors are detected.
*/
declare export default function instanceOf(
value: mixed,
constructor: mixed,
): boolean %checks(value instanceof constructor);
48 changes: 48 additions & 0 deletions src/jsutils/instanceOf.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* A replacement for instanceof which includes an error warning when multi-realm
* constructors are detected.
*/

// Keep the message around, but only show it once (per instance of graphql-js that encounters mismatched constructors)
let warned = false;
// See: https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production
// See: https://webpack.js.org/guides/production/
export default process.env.NODE_ENV === 'production'
? // eslint-disable-next-line no-shadow
function instanceOf<T>(
value: unknown,
constructor: new () => T,
): value is T {
return (
value instanceof constructor ||
(value != null &&
value.constructor != null &&
constructor.name != null &&
value.constructor.name == constructor.name)
);
}
: // eslint-disable-next-line no-shadow
function instanceOf<T>(
value: unknown,
constructor: new () => T,
): value is T {
if (value instanceof constructor) {
return true;
}
if (value) {
const valueClass = value.constructor;
const className = constructor.name;
if (className && valueClass && valueClass.name === className) {
if (!warned) {
console.warn(
`@apollo/graphql: Using ${className} "${value}" from another module or realm.
This could produce confusing and spurious results, especially if your "graphql"/"@apollo/graphql" versions are inconsistent.
You can use \`npm ls graphql\` and \`npm ls @apollo/graphql\` to ensure all versions are consistent.`, // TODO: determine what we mean by "consistent"
);
warned = true;
}
return true;
}
}
return false;
};

0 comments on commit 9ae0995

Please sign in to comment.