From c7f879918a95574c0ca0412f123265f802b64ea4 Mon Sep 17 00:00:00 2001 From: Evilebot Tnawi Date: Wed, 17 Jun 2020 17:04:55 +0300 Subject: [PATCH] fix: respect 'use strict' (#77) --- README.md | 6 +- package-lock.json | 5 ++ package.json | 3 +- src/index.js | 7 ++- src/utils.js | 49 +++++++-------- test/__snapshots__/loader.test.js.snap | 82 +++++++++++++++++++++++++ test/fixtures/use-strict-in-function.js | 7 +++ test/fixtures/use-strict.js | 11 ++++ test/loader.test.js | 50 +++++++++++++++ 9 files changed, 189 insertions(+), 31 deletions(-) create mode 100644 test/fixtures/use-strict-in-function.js create mode 100644 test/fixtures/use-strict.js diff --git a/README.md b/README.md index f4b8f10..7b14593 100644 --- a/README.md +++ b/README.md @@ -486,6 +486,8 @@ import $ from 'jquery'; }.call(window, document)); ``` +> ⚠ Do not use this option if source code contains ES module import(s) + ### additionalCode Type: `String` @@ -507,7 +509,7 @@ module.exports = { moduleName: 'jquery', name: '$', }, - additionalCode: 'var someVariable = 1;', + additionalCode: 'var define = false;', }, }, ], @@ -522,7 +524,7 @@ Generate output: ```js import $ from 'jquery'; -var someVariable = 1; +var define = false; // ... // Code diff --git a/package-lock.json b/package-lock.json index a8656a5..0c481fd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12835,6 +12835,11 @@ "integrity": "sha1-IzTBjpx1n3vdVv3vfprj1YjmjtM=", "dev": true }, + "strip-comments": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/strip-comments/-/strip-comments-2.0.1.tgz", + "integrity": "sha512-ZprKx+bBLXv067WTCALv8SSz5l2+XhpYCsVtSqlMnkAXMWDq+/ekVbl1ghqP9rUHTzv6sm/DwCOiYutU/yp1fw==" + }, "strip-eof": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", diff --git a/package.json b/package.json index aae5fd4..65268c1 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,8 @@ "dependencies": { "loader-utils": "^2.0.0", "schema-utils": "^2.7.0", - "source-map": "^0.6.1" + "source-map": "^0.6.1", + "strip-comments": "^2.0.1" }, "devDependencies": { "@babel/cli": "^7.10.1", diff --git a/src/index.js b/src/index.js index 2247339..7899a91 100644 --- a/src/index.js +++ b/src/index.js @@ -9,7 +9,7 @@ import validateOptions from 'schema-utils'; import schema from './options.json'; -import { getImports, renderImports } from './utils'; +import { getImports, renderImports, sourceHasUseStrict } from './utils'; const HEADER = '/*** IMPORTS FROM imports-loader ***/\n'; @@ -37,10 +37,13 @@ export default function loader(content, sourceMap) { return; } + // We don't need to remove 'use strict' manually, because `terser` do it + const directive = sourceHasUseStrict(content); + importsCode += Object.entries(imports).reduce( (accumulator, item) => `${accumulator}${renderImports(this, type, item[0], item[1])}\n`, - '' + directive ? "'use strict';\n" : '' ); } diff --git a/src/utils.js b/src/utils.js index a633569..f398a58 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,4 +1,17 @@ import { stringifyRequest } from 'loader-utils'; +import strip from 'strip-comments'; + +function forError(item) { + return typeof item === 'string' + ? item + : `\n${JSON.stringify(item, null, ' ')}\n`; +} + +function sourceHasUseStrict(source) { + const str = strip(source).trim(); + + return str.startsWith("'use strict'") || str.startsWith('"use strict"'); +} function resolveImports(type, item) { const defaultSyntax = type === 'module' ? 'default' : 'single'; @@ -50,11 +63,7 @@ function resolveImports(type, item) { throw new Error( `The "${result.syntax}" syntax does not support "${ result.alias - }" alias in "${ - typeof item === 'string' - ? item - : `\n${JSON.stringify(item, null, ' ')}\n` - }" value` + }" alias in "${forError(item)}" value` ); } @@ -65,11 +74,7 @@ function resolveImports(type, item) { throw new Error( `The "${result.syntax}" syntax does not support "${ result.name - }" name in "${ - typeof item === 'string' - ? item - : `\n${JSON.stringify(item, null, ' ')}\n` - }" value` + }" name in "${forError(item)}" value` ); } @@ -78,11 +83,9 @@ function resolveImports(type, item) { type === 'commonjs' ) { throw new Error( - `The "${type}" type does not support the "${result.syntax}" syntax in "${ - typeof item === 'string' - ? item - : `\n${JSON.stringify(item, null, ' ')}\n` - }" value` + `The "${type}" type does not support the "${ + result.syntax + }" syntax in "${forError(item)}" value` ); } @@ -93,11 +96,7 @@ function resolveImports(type, item) { throw new Error( `The "${type}" format does not support the "${ result.syntax - }" syntax in "${ - typeof item === 'string' - ? item - : `\n${JSON.stringify(item, null, ' ')}\n` - }" value` + }" syntax in "${forError(item)}" value` ); } @@ -106,11 +105,9 @@ function resolveImports(type, item) { typeof result.name === 'undefined' ) { throw new Error( - `The "${result.syntax}" syntax needs the "name" option in "${ - typeof item === 'string' - ? item - : `\n${JSON.stringify(item, null, ' ')}\n` - }" value` + `The "${result.syntax}" syntax needs the "name" option in "${forError( + item + )}" value` ); } @@ -315,4 +312,4 @@ function renderImports(loaderContext, type, moduleName, imports) { return code; } -export { getImports, renderImports }; +export { sourceHasUseStrict, getImports, renderImports }; diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index f7805b0..8663806 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -649,6 +649,88 @@ var someCode = { exports[`loader should work with "single" imports without syntax: warnings 1`] = `Array []`; +exports[`loader should work with "use-strict" not in program with CommonJS modules: errors 1`] = `Array []`; + +exports[`loader should work with "use-strict" not in program with CommonJS modules: module 1`] = ` +"/*** IMPORTS FROM imports-loader ***/ +var lib_1 = require(\\"lib_1\\"); + +var myObject = { foo: 'bar' }; + +function test() { + 'use strict'; + + return 'test'; +} +" +`; + +exports[`loader should work with "use-strict" not in program with CommonJS modules: warnings 1`] = `Array []`; + +exports[`loader should work with "use-strict" not in program with ES modules: errors 1`] = `Array []`; + +exports[`loader should work with "use-strict" not in program with ES modules: module 1`] = ` +"/*** IMPORTS FROM imports-loader ***/ +import lib_1 from \\"lib_1\\"; + +var myObject = { foo: 'bar' }; + +function test() { + 'use strict'; + + return 'test'; +} +" +`; + +exports[`loader should work with "use-strict" not in program with ES modules: warnings 1`] = `Array []`; + +exports[`loader should work with "use-strict" with CommonJS modules: errors 1`] = `Array []`; + +exports[`loader should work with "use-strict" with CommonJS modules: module 1`] = ` +"/*** IMPORTS FROM imports-loader ***/ +'use strict'; +var lib_1 = require(\\"lib_1\\"); + +/* Comment */ + +'use strict'; + +var myObject = { foo: 'bar' }; + +function test() { + 'use strict'; + + return 'test'; +} +" +`; + +exports[`loader should work with "use-strict" with CommonJS modules: warnings 1`] = `Array []`; + +exports[`loader should work with "use-strict" with ES modules: errors 1`] = `Array []`; + +exports[`loader should work with "use-strict" with ES modules: module 1`] = ` +"/*** IMPORTS FROM imports-loader ***/ +'use strict'; +import lib_1 from \\"lib_1\\"; + +/* Comment */ + +'use strict'; + +var myObject = { foo: 'bar' }; + +function test() { + 'use strict'; + + return 'test'; +} +" +`; + +exports[`loader should work with "use-strict" with ES modules: warnings 1`] = `Array []`; + exports[`loader should work with a string syntax using CommonJS: errors 1`] = `Array []`; exports[`loader should work with a string syntax using CommonJS: module 1`] = ` diff --git a/test/fixtures/use-strict-in-function.js b/test/fixtures/use-strict-in-function.js new file mode 100644 index 0000000..44dcc4d --- /dev/null +++ b/test/fixtures/use-strict-in-function.js @@ -0,0 +1,7 @@ +var myObject = { foo: 'bar' }; + +function test() { + 'use strict'; + + return 'test'; +} diff --git a/test/fixtures/use-strict.js b/test/fixtures/use-strict.js new file mode 100644 index 0000000..ade9149 --- /dev/null +++ b/test/fixtures/use-strict.js @@ -0,0 +1,11 @@ +/* Comment */ + +'use strict'; + +var myObject = { foo: 'bar' }; + +function test() { + 'use strict'; + + return 'test'; +} diff --git a/test/loader.test.js b/test/loader.test.js index e827438..f808980 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -149,6 +149,56 @@ describe('loader', () => { expect(getWarnings(stats)).toMatchSnapshot('warnings'); }); + it('should work with "use-strict" with ES modules', async () => { + const compiler = getCompiler('use-strict.js', { + imports: 'lib_1', + }); + const stats = await compile(compiler); + + expect(getModuleSource('./use-strict.js', stats)).toMatchSnapshot('module'); + expect(getErrors(stats)).toMatchSnapshot('errors'); + expect(getWarnings(stats)).toMatchSnapshot('warnings'); + }); + + it('should work with "use-strict" with CommonJS modules', async () => { + const compiler = getCompiler('use-strict.js', { + type: 'commonjs', + imports: 'lib_1', + }); + const stats = await compile(compiler); + + expect(getModuleSource('./use-strict.js', stats)).toMatchSnapshot('module'); + expect(getErrors(stats)).toMatchSnapshot('errors'); + expect(getWarnings(stats)).toMatchSnapshot('warnings'); + }); + + it('should work with "use-strict" not in program with ES modules', async () => { + const compiler = getCompiler('use-strict-in-function.js', { + imports: 'lib_1', + }); + const stats = await compile(compiler); + + expect( + getModuleSource('./use-strict-in-function.js', stats) + ).toMatchSnapshot('module'); + expect(getErrors(stats)).toMatchSnapshot('errors'); + expect(getWarnings(stats)).toMatchSnapshot('warnings'); + }); + + it('should work with "use-strict" not in program with CommonJS modules', async () => { + const compiler = getCompiler('use-strict-in-function.js', { + type: 'commonjs', + imports: 'lib_1', + }); + const stats = await compile(compiler); + + expect( + getModuleSource('./use-strict-in-function.js', stats) + ).toMatchSnapshot('module'); + expect(getErrors(stats)).toMatchSnapshot('errors'); + expect(getWarnings(stats)).toMatchSnapshot('warnings'); + }); + it('should work with "imports", "wrapper" and "additionalCode" options', async () => { const compiler = getCompiler('some-library.js', { imports: {