From 4ff9d2f3bd42c14a30a0e994f6c7866fb7c33850 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 3 Dec 2020 10:43:12 -0500 Subject: [PATCH 1/8] Add failing test for when renamed imports are not transpiled to the correct thing --- fixtures/assert-expansion/expectation6.js | 3 ++- fixtures/assert-expansion/expectation7.js | 3 ++- fixtures/assert-expansion/sample.js | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/fixtures/assert-expansion/expectation6.js b/fixtures/assert-expansion/expectation6.js index 3aab006..345bb9c 100644 --- a/fixtures/assert-expansion/expectation6.js +++ b/fixtures/assert-expansion/expectation6.js @@ -1,2 +1,3 @@ (true && !((() => true)()) && console.assert((() => true)(), 'This is an assertion')); -(true && !(false) && console.assert(false, 'This is an assertion 2')); \ No newline at end of file +(true && !(false) && console.assert(false, 'This is an assertion 2')); +(true && !(false) && console.assert(false, 'renamed assertion')); diff --git a/fixtures/assert-expansion/expectation7.js b/fixtures/assert-expansion/expectation7.js index 3aab006..345bb9c 100644 --- a/fixtures/assert-expansion/expectation7.js +++ b/fixtures/assert-expansion/expectation7.js @@ -1,2 +1,3 @@ (true && !((() => true)()) && console.assert((() => true)(), 'This is an assertion')); -(true && !(false) && console.assert(false, 'This is an assertion 2')); \ No newline at end of file +(true && !(false) && console.assert(false, 'This is an assertion 2')); +(true && !(false) && console.assert(false, 'renamed assertion')); diff --git a/fixtures/assert-expansion/sample.js b/fixtures/assert-expansion/sample.js index c417594..423de62 100644 --- a/fixtures/assert-expansion/sample.js +++ b/fixtures/assert-expansion/sample.js @@ -1,5 +1,7 @@ import { DEBUG } from '@ember/env-flags'; import { assert } from '@ember/debug-tools'; +import { assert as debugAssert } from '@ember/debug-tools'; assert((() => true )(), 'This is an assertion'); -assert(false, 'This is an assertion 2'); \ No newline at end of file +assert(false, 'This is an assertion 2'); +debugAssert(false, 'renamed assertion'); From 71f4e2d84a41de5363fae8f2effc8371a6436e24 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 3 Dec 2020 11:09:26 -0500 Subject: [PATCH 2/8] attempt at fix. partial --- src/utils/macros.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/utils/macros.js b/src/utils/macros.js index 46bdddc..85a3333 100644 --- a/src/utils/macros.js +++ b/src/utils/macros.js @@ -34,7 +34,7 @@ module.exports = class Macros { collectDebugToolsSpecifiers(specifiers) { specifiers.forEach(specifier => { if (specifier.node.imported && SUPPORTED_MACROS.indexOf(specifier.node.imported.name) > -1) { - this.localDebugBindings.push(specifier.get('local')); + this.localDebugBindings.push(specifier); } }); } @@ -47,7 +47,7 @@ module.exports = class Macros { if ( this.builder.t.isCallExpression(expression) && - this.localDebugBindings.some(b => b.node.name === expression.callee.name) + this.localDebugBindings.some(b => b.get('imported').node.name === expression.callee.name) ) { let imported = path.scope.getBinding(expression.callee.name).path.node.imported.name; this.builder[`${imported}`](path); @@ -65,9 +65,11 @@ module.exports = class Macros { let specifiers = importPath.get('specifiers'); if (specifiers.length === this.localDebugBindings.length) { - this.localDebugBindings[0].parentPath.parentPath.remove(); + importPath.remove(); + } else if (specifiers.length === 1) { + importPath.remove(); } else { - this.localDebugBindings.forEach(binding => binding.parentPath.remove()); + this.localDebugBindings.forEach(binding => binding.get('local').parentPath.remove()); } } } From c5f74d45064819f0fa2fb05f02bb4570095ae53b Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 3 Dec 2020 11:37:37 -0500 Subject: [PATCH 3/8] note that the builder needs changings --- src/utils/macros.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/macros.js b/src/utils/macros.js index 85a3333..c8cd53f 100644 --- a/src/utils/macros.js +++ b/src/utils/macros.js @@ -47,9 +47,10 @@ module.exports = class Macros { if ( this.builder.t.isCallExpression(expression) && - this.localDebugBindings.some(b => b.get('imported').node.name === expression.callee.name) + this.localDebugBindings.some(b => b.get('local').node.name === expression.callee.name) ) { let imported = path.scope.getBinding(expression.callee.name).path.node.imported.name; + // The builder needs to be made aware of the the local name of the ImportSpecifier this.builder[`${imported}`](path); } } @@ -62,12 +63,11 @@ module.exports = class Macros { // import declaration in question seems to have already been removed return; } + let specifiers = importPath.get('specifiers'); if (specifiers.length === this.localDebugBindings.length) { importPath.remove(); - } else if (specifiers.length === 1) { - importPath.remove(); } else { this.localDebugBindings.forEach(binding => binding.get('local').parentPath.remove()); } From 152b6ac7f9d980d21c3a2674966c7373a5b57610 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Sun, 21 Feb 2021 23:24:21 -0500 Subject: [PATCH 4/8] just have stray imports left --- src/utils/builder.js | 8 ++++++-- src/utils/macros.js | 12 +++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/utils/builder.js b/src/utils/builder.js index ea61084..4ba6cbd 100644 --- a/src/utils/builder.js +++ b/src/utils/builder.js @@ -27,7 +27,7 @@ module.exports = class Builder { * * ($DEBUG && $GLOBAL_NS.assert($PREDICATE, $MESSAGE)); */ - assert(path) { + assert(path, options) { let predicate; if (this.assertPredicateIndex !== undefined) { predicate = (expression, args) => { @@ -37,6 +37,7 @@ module.exports = class Builder { this._createMacroExpression(path, { predicate, + ...options, }); } @@ -102,7 +103,10 @@ module.exports = class Builder { } else if (options.buildConsoleAPI) { callExpression = options.buildConsoleAPI(expression, args); } else { - callExpression = this._createConsoleAPI(options.consoleAPI || callee, args); + callExpression = this._createConsoleAPI( + options.consoleAPI || (options.importedName && t.identifier(options.importedName)) || callee, + args + ); } let prefixedIdentifiers = []; diff --git a/src/utils/macros.js b/src/utils/macros.js index c8cd53f..2c6d1a5 100644 --- a/src/utils/macros.js +++ b/src/utils/macros.js @@ -49,9 +49,13 @@ module.exports = class Macros { this.builder.t.isCallExpression(expression) && this.localDebugBindings.some(b => b.get('local').node.name === expression.callee.name) ) { - let imported = path.scope.getBinding(expression.callee.name).path.node.imported.name; + let specifier = path.scope.getBinding(expression.callee.name).path.node; + let imported = specifier.imported.name; // The builder needs to be made aware of the the local name of the ImportSpecifier - this.builder[`${imported}`](path); + this.builder[`${imported}`](path, { + localName: specifier.local.name, + importedName: imported, + }); } } @@ -69,7 +73,9 @@ module.exports = class Macros { if (specifiers.length === this.localDebugBindings.length) { importPath.remove(); } else { - this.localDebugBindings.forEach(binding => binding.get('local').parentPath.remove()); + this.localDebugBindings.forEach(binding => { + binding.get('local').parentPath.remove(); + }); } } } From 9f42ebf510e87a9a45fb02ef7f1fc61c9545c61b Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 22 Feb 2021 15:53:55 -0500 Subject: [PATCH 5/8] finish fix --- src/utils/macros.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/utils/macros.js b/src/utils/macros.js index 2c6d1a5..9b929b9 100644 --- a/src/utils/macros.js +++ b/src/utils/macros.js @@ -74,7 +74,14 @@ module.exports = class Macros { importPath.remove(); } else { this.localDebugBindings.forEach(binding => { - binding.get('local').parentPath.remove(); + let specifier = binding.get('local').parentPath; + let importPath = specifier.parentPath; + + if (importPath.get('specifiers').length === 1) { + importPath.remove(); + } else { + specifier.remove(); + } }); } } From 1fa2d083e7d49aa402bc9a99b8a8bfabaf73b13e Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 22 Feb 2021 15:54:12 -0500 Subject: [PATCH 6/8] make trailing and leading blankspace irrelevant in testing fixtures --- tests/create-tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/create-tests.js b/tests/create-tests.js index dbf09f0..7590e1f 100644 --- a/tests/create-tests.js +++ b/tests/create-tests.js @@ -453,7 +453,7 @@ function createTests(options) { `./fixtures/${fixtureName}/expectation${babelVersion}.js`, 'utf-8' ); - expect(transform(sample, options).code).toEqual(expectation); + expect(transform(sample, options).code.trim()).toEqual(expectation.trim()); }); }, From 76fb7db063c6cf9b72bf77ed3fc34c2d47088ef0 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 22 Feb 2021 15:56:52 -0500 Subject: [PATCH 7/8] support node 10 --- src/utils/builder.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/utils/builder.js b/src/utils/builder.js index 4ba6cbd..4018132 100644 --- a/src/utils/builder.js +++ b/src/utils/builder.js @@ -35,10 +35,15 @@ module.exports = class Builder { }; } - this._createMacroExpression(path, { - predicate, - ...options, - }); + this._createMacroExpression( + path, + Object.assign( + { + predicate, + }, + options + ) + ); } /** @@ -104,7 +109,9 @@ module.exports = class Builder { callExpression = options.buildConsoleAPI(expression, args); } else { callExpression = this._createConsoleAPI( - options.consoleAPI || (options.importedName && t.identifier(options.importedName)) || callee, + options.consoleAPI || + (options.importedName && t.identifier(options.importedName)) || + callee, args ); } From d97c4edcf9269f6b09d5f7e46bfe5aa316dfb1cf Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Mon, 8 Mar 2021 10:25:34 -0500 Subject: [PATCH 8/8] Add failing tests for Ember Replacement --- fixtures/ember-cli-babel-config/expectation6.js | 13 ++++++++++++- fixtures/ember-cli-babel-config/expectation7.js | 13 ++++++++++++- fixtures/ember-cli-babel-config/sample.js | 16 +++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/fixtures/ember-cli-babel-config/expectation6.js b/fixtures/ember-cli-babel-config/expectation6.js index 8995e5d..47bd063 100644 --- a/fixtures/ember-cli-babel-config/expectation6.js +++ b/fixtures/ember-cli-babel-config/expectation6.js @@ -11,4 +11,15 @@ if (true /* DEBUG */) { id: 'donzo', until: '4.0.0', url: 'http://example.com' -})); \ No newline at end of file +})); + +// renamed imports + +(true && Ember.warn('This is a warning')); +(true && !(foo) && Ember.assert('Hahahaha', foo)); +(true && !(false) && Ember.assert('without predicate')); +(true && !(true) && Ember.deprecate('This thing is donzo', true, { + id: 'donzo', + until: '4.0.0', + url: 'http://example.com' +})); diff --git a/fixtures/ember-cli-babel-config/expectation7.js b/fixtures/ember-cli-babel-config/expectation7.js index 8b757cd..71ab2f9 100644 --- a/fixtures/ember-cli-babel-config/expectation7.js +++ b/fixtures/ember-cli-babel-config/expectation7.js @@ -11,4 +11,15 @@ if (true id: 'donzo', until: '4.0.0', url: 'http://example.com' -})); \ No newline at end of file +})); + +// renamed imports + +(true && Ember.warn('This is a warning')); +(true && !(foo) && Ember.assert('Hahahaha', foo)); +(true && !(false) && Ember.assert('without predicate')); +(true && !(true) && Ember.deprecate('This thing is donzo', true, { + id: 'donzo', + until: '4.0.0', + url: 'http://example.com' +})); // renamed imports diff --git a/fixtures/ember-cli-babel-config/sample.js b/fixtures/ember-cli-babel-config/sample.js index c291e51..7b6a14b 100644 --- a/fixtures/ember-cli-babel-config/sample.js +++ b/fixtures/ember-cli-babel-config/sample.js @@ -1,4 +1,4 @@ -import { warn, assert, deprecate } from '@ember/debug'; +import { warn, assert, deprecate, warn as debugWarn, assert as debugAssert, deprecate as debugDeprecate } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; if (DEBUG) { @@ -15,3 +15,17 @@ deprecate('This thing is donzo', true, { until: '4.0.0', url: 'http://example.com' }); + +// renamed imports +debugWarn('This is a warning'); + +debugAssert('Hahahaha', foo); +debugAssert('without predicate'); + +debugDeprecate('This thing is donzo', true, { + id: 'donzo', + until: '4.0.0', + url: 'http://example.com' +}); + +