From 83291c839fde5f568f93b0434496113e20ff6e19 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 21 Mar 2022 11:47:24 +0100 Subject: [PATCH 1/2] fix: `isConstruct` is wrong when symlinking libraries `Construct.isConstruct` was still using (and recommending) `instanceof`, even though `instanceof` can never be made to work reliably. When we thought `instanceof` was safe to use again, it's because we thought that `npm install` combined with `peerDependencies` would make sure only one copy of `constructs` would ever be installed. That's correct, but monorepos and users using `npm link` can still mess up that guarantee, so we cannot rely on it after all. --- src/construct.ts | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/construct.ts b/src/construct.ts index 021d2220..2e7444f2 100644 --- a/src/construct.ts +++ b/src/construct.ts @@ -3,6 +3,8 @@ import { MetadataEntry } from './metadata'; import { captureStackTrace } from './private/stack-trace'; import { addressOf } from './private/uniqueid'; +const CONSTRUCT_SYM = Symbol.for('constructs.Construct'); + /** * Represents a construct. */ @@ -422,13 +424,26 @@ export class Node { export class Construct implements IConstruct { /** * Checks if `x` is a construct. + * + * Use this method instead of `instanceof` to properly detect `Construct` + * instances, even when the construct library is symlinked. + * + * Explanation: in JavaScript, multiple copies of the `constructs` library on + * disk are seen as independent, completely different libraries. As a + * consequence, the class `Construct` in each copy of the `constructs` library + * is seen as a different class, and an instance of one class will not test as + * `instanceof` the other class. `npm install` will not create installations + * like this, but users may manually symlink construct libraries together or + * use a monorepo tool: in those cases, multiple copies of the `constructs` + * library can be accidentally installed, and `instanceof` will behave + * unpredictably. It is safest to avoid using `instanceof`, and using + * this type-testing method instead. + * * @returns true if `x` is an object created from a class which extends `Construct`. * @param x Any object - * - * @deprecated use `x instanceof Construct` instead */ public static isConstruct(x: any): x is Construct { - return x instanceof Construct; + return x && typeof x === 'object' && x[CONSTRUCT_SYM]; } /** @@ -536,3 +551,10 @@ export interface MetadataOptions { */ readonly traceFromFunction?: any; } + +// Mark all instances of 'Construct' +Object.defineProperty(Construct.prototype, CONSTRUCT_SYM, { + value: true, + enumerable: false, + writable: false, +}); \ No newline at end of file From b73cfa52e913f095b2286b822b9e751533f0c560 Mon Sep 17 00:00:00 2001 From: github-actions Date: Mon, 21 Mar 2022 10:51:45 +0000 Subject: [PATCH 2/2] chore: self mutation Signed-off-by: github-actions --- API.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/API.md b/API.md index 512cd80c..1c05813e 100644 --- a/API.md +++ b/API.md @@ -80,10 +80,24 @@ toString(): string __Returns__: * string -#### *static* isConstruct(x)⚠️ +#### *static* isConstruct(x) Checks if `x` is a construct. +Use this method instead of `instanceof` to properly detect `Construct` +instances, even when the construct library is symlinked. + +Explanation: in JavaScript, multiple copies of the `constructs` library on +disk are seen as independent, completely different libraries. As a +consequence, the class `Construct` in each copy of the `constructs` library +is seen as a different class, and an instance of one class will not test as +`instanceof` the other class. `npm install` will not create installations +like this, but users may manually symlink construct libraries together or +use a monorepo tool: in those cases, multiple copies of the `constructs` +library can be accidentally installed, and `instanceof` will behave +unpredictably. It is safest to avoid using `instanceof`, and using +this type-testing method instead. + ```ts static isConstruct(x: any): boolean ```