From 9b3884993e26abcf3df5d611670e4342441f19cf Mon Sep 17 00:00:00 2001 From: Stanley Stuart Date: Thu, 28 May 2020 23:06:14 -0700 Subject: [PATCH 1/3] [bugfix] don't attempt to replace typescript type identifiers [#109](5de3c790bc6f037421961269bd5df10fd032b7ac) introduced a safer way to rewrite variable names by generating MemberExpressions to fix code for IE9, but this introduced a regression for packages using `@babel/plugin-transform-typescript`, such as Ember Data. After 109, Babel would throw the following error if `babel-plugin-ember-modules-api-polyfill` attempted to rewrite a type information node (in this case, `TSQualifiedName`), if the file also used the import in runtime JS code. The code that triggers this looks like: ```javascript import { default as RSVP, Promise } from 'rsvp'; // value must be used to trigger this RSVP.Promise.resolve().then(() => {}); // AND the same import local name must be used as a type as well function scheduleSave(identifier: RecordIdentifier, options: any = {}): RSVP.Promise { } ``` ``` Property left of TSQualifiedName expected node to be of a type ["TSEntityName"] but instead got "MemberExpression" ``` This was being triggered when `ember-data` was compiled. Examples were taken from Ember Data's codebase: - https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L124 - https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L124 - https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L77 This change looks at the `type` of the `referencePath.parentNode` to see if it belongs with `TS`, since it seems like all Typescript types in @babel/types are namespaced with `TS`. There's unfortunately no list or API of typescript nodes we could programatically pull this from. Since type information is removed from Babel output, and babel does not do any linting/type checking of transformed Typescript, it seems fine to leave these nodes alone. --- __tests__/index-test.js | 24 ++++++++++++++++++++++++ package.json | 1 + src/index.js | 6 +++++- yarn.lock | 16 ++++++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/__tests__/index-test.js b/__tests__/index-test.js index 9de27c3..4b681af 100644 --- a/__tests__/index-test.js +++ b/__tests__/index-test.js @@ -267,3 +267,27 @@ describe('when used with @babel/preset-env', () => { expect(actual).toEqual(`export default Ember.Application.extend({});`); }); }); + +describe('when used with typescript', () => { + // Example taken from Ember Data: + // https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L124 + // https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L124 + // https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L77 + it(`works when you use an import as both a type and as JS value`, () => { + let source = ` + import { default as RSVP, Promise } from 'rsvp'; + RSVP.Promise.resolve().then(() => {}); + function scheduleSave(identifier: RecordIdentifier, options: any = {}): RSVP.Promise { + } + `; + + let actual = transform7(source, [ + require('@babel/plugin-transform-typescript'), + Plugin, + ]); + + expect(actual).toEqual( + `Ember.RSVP.Promise.resolve().then(() => {});\n\nfunction scheduleSave(identifier, options = {}) {}` + ); + }); +}); diff --git a/package.json b/package.json index 2849656..12282ea 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ }, "devDependencies": { "@babel/core": "^7.10.1", + "@babel/plugin-transform-typescript": "^7.10.1", "@babel/preset-env": "^7.10.1", "babel-core": "^6.25.0", "eslint": "^7.1.0", diff --git a/src/index.js b/src/index.js index da3c938..964fc78 100644 --- a/src/index.js +++ b/src/index.js @@ -16,6 +16,8 @@ function isBlacklisted(blacklist, importPath, exportName) { module.exports = function (babel) { const t = babel.types; + const isTypescriptNode = (node) => node.type.indexOf('TS') >= 0; + const GLOBALS_MAP = new Map(); // Flips the ember-rfc176-data mapping into an 'import' indexed object, that exposes the @@ -167,7 +169,9 @@ module.exports = function (babel) { let binding = path.scope.getBinding(local.name); binding.referencePaths.forEach((referencePath) => { - referencePath.replaceWith(getMemberExpressionFor(global)); + if (!isTypescriptNode(referencePath.parentPath)) { + referencePath.replaceWith(getMemberExpressionFor(global)); + } }); } }); diff --git a/yarn.lock b/yarn.lock index 843235d..1a0fc98 100644 --- a/yarn.lock +++ b/yarn.lock @@ -433,6 +433,13 @@ dependencies: "@babel/helper-plugin-utils" "^7.10.1" +"@babel/plugin-syntax-typescript@^7.10.1": + version "7.10.1" + resolved "https://registry.yarnpkg.com/@babel/plugin-syntax-typescript/-/plugin-syntax-typescript-7.10.1.tgz#5e82bc27bb4202b93b949b029e699db536733810" + integrity sha512-X/d8glkrAtra7CaQGMiGs/OGa6XgUzqPcBXCIGFCpCqnfGlT0Wfbzo/B89xHhnInTaItPK8LALblVXcUOEh95Q== + dependencies: + "@babel/helper-plugin-utils" "^7.10.1" + "@babel/plugin-transform-arrow-functions@^7.10.1": version "7.10.1" resolved "https://registry.yarnpkg.com/@babel/plugin-transform-arrow-functions/-/plugin-transform-arrow-functions-7.10.1.tgz#cb5ee3a36f0863c06ead0b409b4cc43a889b295b" @@ -669,6 +676,15 @@ dependencies: "@babel/helper-plugin-utils" "^7.10.1" +"@babel/plugin-transform-typescript@^7.10.1": + version "7.10.1" + resolved "https://registry.yarnpkg.com/@babel/plugin-transform-typescript/-/plugin-transform-typescript-7.10.1.tgz#2c54daea231f602468686d9faa76f182a94507a6" + integrity sha512-v+QWKlmCnsaimLeqq9vyCsVRMViZG1k2SZTlcZvB+TqyH570Zsij8nvVUZzOASCRiQFUxkLrn9Wg/kH0zgy5OQ== + dependencies: + "@babel/helper-create-class-features-plugin" "^7.10.1" + "@babel/helper-plugin-utils" "^7.10.1" + "@babel/plugin-syntax-typescript" "^7.10.1" + "@babel/plugin-transform-unicode-escapes@^7.10.1": version "7.10.1" resolved "https://registry.yarnpkg.com/@babel/plugin-transform-unicode-escapes/-/plugin-transform-unicode-escapes-7.10.1.tgz#add0f8483dab60570d9e03cecef6c023aa8c9940" From 5a47579c7c53511222f2f5cae30232570d7b6f64 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 29 May 2020 09:22:47 -0400 Subject: [PATCH 2/3] Add test for @glimmer/component style failure mode. --- __tests__/index-test.js | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/__tests__/index-test.js b/__tests__/index-test.js index 4b681af..4d21287 100644 --- a/__tests__/index-test.js +++ b/__tests__/index-test.js @@ -273,7 +273,7 @@ describe('when used with typescript', () => { // https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L124 // https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L124 // https://github.com/emberjs/data/blob/70b0c55e1a950bed1da64d0ecb4eaa0d5df92f9f/packages/store/addon/-private/system/fetch-manager.ts#L77 - it(`works when you use an import as both a type and as JS value`, () => { + it(`works when you use an import as both a type and as a TSQualifiedName value`, () => { let source = ` import { default as RSVP, Promise } from 'rsvp'; RSVP.Promise.resolve().then(() => {}); @@ -290,4 +290,22 @@ describe('when used with typescript', () => { `Ember.RSVP.Promise.resolve().then(() => {});\n\nfunction scheduleSave(identifier, options = {}) {}` ); }); + + it(`works when you use an import as both a type and a TSDeclareFunction`, () => { + let source = ` + import { capabilities } from '@ember/component'; + + declare module '@ember/component' { + export function capabilities( + ); + } + `; + + let actual = transform7(source, [ + require('@babel/plugin-transform-typescript'), + Plugin, + ]); + + expect(actual).toEqual(``); + }); }); From c01c7189d6321787babf5fa4cf70ffbb6cab8545 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 29 May 2020 09:23:03 -0400 Subject: [PATCH 3/3] Use `.startsWith` (vs checking for TS anywhere in the type) --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 964fc78..4c41215 100644 --- a/src/index.js +++ b/src/index.js @@ -16,7 +16,7 @@ function isBlacklisted(blacklist, importPath, exportName) { module.exports = function (babel) { const t = babel.types; - const isTypescriptNode = (node) => node.type.indexOf('TS') >= 0; + const isTypescriptNode = (node) => node.type.startsWith('TS'); const GLOBALS_MAP = new Map();