From 6e2f2f8af62260e61ddec95ac4ed6b6ab07ca9d2 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 1 Feb 2021 21:03:47 -0500 Subject: [PATCH 1/2] Add option to preserve import Ember from 'ember'; --- __tests__/index-test.js | 31 +++++++++++++++++++++++++++++ src/index.js | 44 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/__tests__/index-test.js b/__tests__/index-test.js index cfa37f6..9159814 100644 --- a/__tests__/index-test.js +++ b/__tests__/index-test.js @@ -241,6 +241,37 @@ describe('options', () => { expect(actual).toEqual(`var _x = Ember.assert;var _y = Ember.inspect;`); }); }); + + describe('useEmberModule', () => { + it(`adds the ember import when used in sub-modules`, () => { + let input = `import Component from '@ember/component';export default class extends Component {}`; + let actual = transform(input, [[Plugin, { useEmberModule: true }]]); + let expected = `import ${Plugin.uniqueishGlobalName} from 'ember';\nexport default class extends Ember.Component {}`; + + expect(actual).toEqual(expected); + }); + + it(`keeps the ember import`, () => { + let input = `import Ember from 'ember';let x = Ember;`; + let actual = transform(input, [[Plugin, { useEmberModule: true }]]); + + expect(actual).toEqual(input); + }); + + it(`keeps the ember import when renamed`, () => { + let input = `import BestFramework from 'ember';let x = BestFramework;`; + let actual = transform(input, [[Plugin, { useEmberModule: true }]]); + + expect(actual).toEqual(input); + }); + + it(`import then export`, () => { + let input = `import mbr from 'ember';export const Ember = mbr;`; + let actual = transform(input, [[Plugin, { useEmberModule: true }]]); + + expect(actual).toEqual(input); + }); + }); }); describe(`import from 'ember'`, () => { diff --git a/src/index.js b/src/index.js index 5eb8703..baaf4f7 100644 --- a/src/index.js +++ b/src/index.js @@ -31,6 +31,16 @@ function isDecorator(moduleName, importName) { } } +// what are the odds someone else defines this? +const EmberGlobalImportName = '____EMBER_GLOBAL____'; + +function emberImport(t) { + return t.importDeclaration( + [t.importDefaultSpecifier(t.identifier(EmberGlobalImportName))], + t.stringLiteral('ember') + ); +} + module.exports = function (babel) { const t = babel.types; @@ -84,8 +94,25 @@ module.exports = function (babel) { return { name: 'ember-modules-api-polyfill', visitor: { + Program(path, state) { + let options = state.opts || {}; + let useEmberModule = options.useEmberModule || false; + + if (!useEmberModule) return; + + let hasEmberImport = path + .get('body') + .filter((n) => n.type === 'ImportDeclaration') + .find((n) => n.get('source').get('value').node === 'ember'); + + if (!hasEmberImport) { + path.unshiftContainer('body', emberImport(t)); + } + }, ImportDeclaration(path, state) { - let ignore = (state.opts && state.opts.ignore) || []; + let options = state.opts || {}; + let ignore = options.ignore || []; + let useEmberModule = options.useEmberModule || false; let node = path.node; let declarations = []; let removals = []; @@ -105,10 +132,17 @@ module.exports = function (babel) { if (specifierPath) { let local = specifierPath.node.local; - if (local.name !== 'Ember') { - path.scope.rename(local.name, 'Ember'); + + if (useEmberModule) { + if (local.name === 'Ember') { + path.scope.rename(EmberGlobalImportName); + } + } else { + if (local.name !== 'Ember') { + path.scope.rename(local.name, 'Ember'); + } + removals.push(specifierPath); } - removals.push(specifierPath); } else { // import 'ember'; path.remove(); @@ -339,3 +373,5 @@ module.exports = function (babel) { // Provide the path to the package's base directory for caching with broccoli // Ref: https://github.com/babel/broccoli-babel-transpiler#caching module.exports.baseDir = () => path.resolve(__dirname, '..'); + +module.exports.uniqueishGlobalName = EmberGlobalImportName; From 61b457e4313960920667c5e352293d7834309a79 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 15 Feb 2021 13:24:31 -0500 Subject: [PATCH 2/2] Refactor to fix up a few issues. * Ensure import is only added if needed * Use `scope.generateUidIdentifier` to create the new local when needed * Ensure the identifier rewriting code uses the correct identifier --- __tests__/index-test.js | 17 +++++++++- src/index.js | 74 +++++++++++++++++++++++++---------------- 2 files changed, 62 insertions(+), 29 deletions(-) diff --git a/__tests__/index-test.js b/__tests__/index-test.js index 9159814..9abe526 100644 --- a/__tests__/index-test.js +++ b/__tests__/index-test.js @@ -243,10 +243,17 @@ describe('options', () => { }); describe('useEmberModule', () => { + it('does not add Ember import when no Ember related imports are needed', () => { + let input = `console.log('hi mom!');`; + let actual = transform(input, [[Plugin, { useEmberModule: true }]]); + + expect(actual).toEqual(input); + }); + it(`adds the ember import when used in sub-modules`, () => { let input = `import Component from '@ember/component';export default class extends Component {}`; let actual = transform(input, [[Plugin, { useEmberModule: true }]]); - let expected = `import ${Plugin.uniqueishGlobalName} from 'ember';\nexport default class extends Ember.Component {}`; + let expected = `import _Ember from 'ember';\nexport default class extends _Ember.Component {}`; expect(actual).toEqual(expected); }); @@ -258,6 +265,14 @@ describe('options', () => { expect(actual).toEqual(input); }); + it(`reuses a pre-existing ember import`, () => { + let input = `import Ember from 'ember'; import Component from '@ember/component'; export default class extends Component {}`; + let actual = transform(input, [[Plugin, { useEmberModule: true }]]); + let expected = `import Ember from 'ember';export default class extends Ember.Component {}`; + + expect(actual).toEqual(expected); + }); + it(`keeps the ember import when renamed`, () => { let input = `import BestFramework from 'ember';let x = BestFramework;`; let actual = transform(input, [[Plugin, { useEmberModule: true }]]); diff --git a/src/index.js b/src/index.js index baaf4f7..6e4a452 100644 --- a/src/index.js +++ b/src/index.js @@ -31,16 +31,6 @@ function isDecorator(moduleName, importName) { } } -// what are the odds someone else defines this? -const EmberGlobalImportName = '____EMBER_GLOBAL____'; - -function emberImport(t) { - return t.importDeclaration( - [t.importDefaultSpecifier(t.identifier(EmberGlobalImportName))], - t.stringLiteral('ember') - ); -} - module.exports = function (babel) { const t = babel.types; @@ -68,14 +58,17 @@ module.exports = function (babel) { reverseMapping[importRoot][importName] = imported; }); - function getMemberExpressionFor(global) { + function getMemberExpressionFor(global, emberIdentifier) { let parts = global.split('.'); let object = parts.shift(); let property = parts.shift(); + let objectIdentifier = + object === 'Ember' ? emberIdentifier : t.identifier(object); + let memberExpression = t.MemberExpression( - t.identifier(object), + objectIdentifier, t.identifier(property) ); @@ -96,23 +89,47 @@ module.exports = function (babel) { visitor: { Program(path, state) { let options = state.opts || {}; - let useEmberModule = options.useEmberModule || false; - - if (!useEmberModule) return; + let useEmberModule = Boolean(options.useEmberModule); - let hasEmberImport = path + let preexistingEmberImportDeclaration = path .get('body') .filter((n) => n.type === 'ImportDeclaration') .find((n) => n.get('source').get('value').node === 'ember'); - if (!hasEmberImport) { - path.unshiftContainer('body', emberImport(t)); + if ( + // an import was found + preexistingEmberImportDeclaration && + // this accounts for `import from 'ember'` without a local identifier + preexistingEmberImportDeclaration.node.specifiers.length > 0 + ) { + state.emberIdentifier = + preexistingEmberImportDeclaration.node.specifiers[0].local; } + + state.ensureEmberImport = () => { + if (!useEmberModule) { + // ensures that we can always assume `state.emberIdentifier` is set + state.emberIdentifier = t.identifier('Ember'); + return; + } + + if (state.emberIdentifier) return; + + state.emberIdentifier = path.scope.generateUidIdentifier('Ember'); + + let emberImport = t.importDeclaration( + [t.importDefaultSpecifier(state.emberIdentifier)], + t.stringLiteral('ember') + ); + + path.unshiftContainer('body', emberImport); + }; }, + ImportDeclaration(path, state) { let options = state.opts || {}; let ignore = options.ignore || []; - let useEmberModule = options.useEmberModule || false; + let useEmberModule = Boolean(options.useEmberModule); let node = path.node; let declarations = []; let removals = []; @@ -133,11 +150,8 @@ module.exports = function (babel) { if (specifierPath) { let local = specifierPath.node.local; - if (useEmberModule) { - if (local.name === 'Ember') { - path.scope.rename(EmberGlobalImportName); - } - } else { + // when `useEmberModule` is set, we don't need to do anything here + if (!useEmberModule) { if (local.name !== 'Ember') { path.scope.rename(local.name, 'Ember'); } @@ -202,12 +216,15 @@ module.exports = function (babel) { removals.push(specifierPath); + // ensure that the Ember global is imported if needed + state.ensureEmberImport(); + if ( path.scope.bindings[local.name].referencePaths.find( (rp) => rp.parent.type === 'ExportSpecifier' ) ) { - // not safe to use path.scope.rename directly + // not safe to use path.scope.rename directly when this identifier is being directly re-exported declarations.push( t.variableDeclaration('var', [ t.variableDeclarator( @@ -249,7 +266,10 @@ module.exports = function (babel) { // Replace the occurrences of the imported name with the global name. referencePaths.forEach((referencePath) => { if (!isTypescriptNode(referencePath.parentPath)) { - const memberExpression = getMemberExpressionFor(global); + const memberExpression = getMemberExpressionFor( + global, + state.emberIdentifier + ); try { referencePath.replaceWith(memberExpression); @@ -373,5 +393,3 @@ module.exports = function (babel) { // Provide the path to the package's base directory for caching with broccoli // Ref: https://github.com/babel/broccoli-babel-transpiler#caching module.exports.baseDir = () => path.resolve(__dirname, '..'); - -module.exports.uniqueishGlobalName = EmberGlobalImportName;