From d266ac342587feb89711f5f6f374cfdb0fe0e6ea Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Fri, 12 Jun 2020 15:20:39 -0700 Subject: [PATCH 1/7] handle single quote escaping in ids --- src/finalisers/amd.ts | 3 ++- src/finalisers/cjs.ts | 10 ++++++---- src/finalisers/es.ts | 15 ++++++++------- src/finalisers/shared/escapeId.ts | 4 ++++ src/finalisers/system.ts | 3 ++- test/form/samples/quote-id/_config.js | 19 +++++++++++++++++++ test/form/samples/quote-id/_expected/amd.js | 16 ++++++++++++++++ test/form/samples/quote-id/_expected/cjs.js | 16 ++++++++++++++++ test/form/samples/quote-id/_expected/es.js | 1 + test/form/samples/quote-id/_expected/iife.js | 17 +++++++++++++++++ .../form/samples/quote-id/_expected/system.js | 18 ++++++++++++++++++ test/form/samples/quote-id/_expected/umd.js | 18 ++++++++++++++++++ test/form/samples/quote-id/main.js | 1 + 13 files changed, 128 insertions(+), 13 deletions(-) create mode 100644 src/finalisers/shared/escapeId.ts create mode 100644 test/form/samples/quote-id/_config.js create mode 100644 test/form/samples/quote-id/_expected/amd.js create mode 100644 test/form/samples/quote-id/_expected/cjs.js create mode 100644 test/form/samples/quote-id/_expected/es.js create mode 100644 test/form/samples/quote-id/_expected/iife.js create mode 100644 test/form/samples/quote-id/_expected/system.js create mode 100644 test/form/samples/quote-id/_expected/umd.js create mode 100644 test/form/samples/quote-id/main.js diff --git a/src/finalisers/amd.ts b/src/finalisers/amd.ts index 1f930e8324c..b313374385e 100644 --- a/src/finalisers/amd.ts +++ b/src/finalisers/amd.ts @@ -2,6 +2,7 @@ import { Bundle as MagicStringBundle } from 'magic-string'; import { NormalizedOutputOptions } from '../rollup/types'; import { INTEROP_NAMESPACE_VARIABLE } from '../utils/variableNames'; import { FinaliserOptions } from './index'; +import { escapeId } from './shared/escapeId'; import { compactEsModuleExport, esModuleExport } from './shared/esModuleExport'; import getExportBlock from './shared/getExportBlock'; import getInteropBlock from './shared/getInteropBlock'; @@ -37,7 +38,7 @@ export default function amd( ) { warnOnBuiltins(warn, dependencies); - const deps = dependencies.map(m => `'${removeExtensionFromRelativeAmdId(m.id)}'`); + const deps = dependencies.map(m => `'${removeExtensionFromRelativeAmdId(escapeId(m.id))}'`); const args = dependencies.map(m => m.name); const n = options.compact ? '' : '\n'; const _ = options.compact ? '' : ' '; diff --git a/src/finalisers/cjs.ts b/src/finalisers/cjs.ts index 4539f2bd24d..05cbad226f2 100644 --- a/src/finalisers/cjs.ts +++ b/src/finalisers/cjs.ts @@ -2,6 +2,7 @@ import { Bundle as MagicStringBundle } from 'magic-string'; import { NormalizedOutputOptions } from '../rollup/types'; import { INTEROP_DEFAULT_VARIABLE, INTEROP_NAMESPACE_VARIABLE } from '../utils/variableNames'; import { FinaliserOptions } from './index'; +import { escapeId } from './shared/escapeId'; import { compactEsModuleExport, esModuleExport } from './shared/esModuleExport'; import getExportBlock from './shared/getExportBlock'; import { getInteropNamespace } from './shared/getInteropNamespace'; @@ -51,21 +52,22 @@ export default function cjs( importBlock += !options.compact || definingVariable ? `;${n}` : ','; } definingVariable = false; - importBlock += `require('${id}')`; + importBlock += `require('${escapeId(id)}')`; } else { importBlock += options.compact && definingVariable ? ',' : `${importBlock ? `;${n}` : ''}${varOrConst} `; definingVariable = true; if (!options.interop || isChunk || !exportsDefault || !namedExportsMode) { - importBlock += `${name}${_}=${_}require('${id}')`; + importBlock += `${name}${_}=${_}require('${escapeId(id)}')`; } else { needsInterop = true; if (exportsNames) - importBlock += `${name}${_}=${_}require('${id}')${ + importBlock += `${name}${_}=${_}require('${escapeId(id)}')${ options.compact ? ',' : `;\n${varOrConst} ` }${name}__default${_}=${_}${INTEROP_DEFAULT_VARIABLE}(${name})`; - else importBlock += `${name}${_}=${_}${INTEROP_DEFAULT_VARIABLE}(require('${id}'))`; + else + importBlock += `${name}${_}=${_}${INTEROP_DEFAULT_VARIABLE}(require('${escapeId(id)}'))`; } } } diff --git a/src/finalisers/es.ts b/src/finalisers/es.ts index 2f3f3977c33..b6ffa61a12e 100644 --- a/src/finalisers/es.ts +++ b/src/finalisers/es.ts @@ -2,6 +2,7 @@ import { Bundle as MagicStringBundle } from 'magic-string'; import { ChunkDependencies, ChunkExports, ImportSpecifier, ReexportSpecifier } from '../Chunk'; import { NormalizedOutputOptions } from '../rollup/types'; import { FinaliserOptions } from './index'; +import { escapeId } from './shared/escapeId'; export default function es( magicString: MagicStringBundle, @@ -26,7 +27,7 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { const importBlock: string[] = []; for (const { id, reexports, imports, name } of dependencies) { if (!reexports && !imports) { - importBlock.push(`import${_}'${id}';`); + importBlock.push(`import${_}'${escapeId(id)}';`); continue; } if (imports) { @@ -43,10 +44,10 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { } } if (starImport) { - importBlock.push(`import${_}*${_}as ${starImport.local} from${_}'${id}';`); + importBlock.push(`import${_}*${_}as ${starImport.local} from${_}'${escapeId(id)}';`); } if (defaultImport && importedNames.length === 0) { - importBlock.push(`import ${defaultImport.local} from${_}'${id}';`); + importBlock.push(`import ${defaultImport.local} from${_}'${escapeId(id)}';`); } else if (importedNames.length > 0) { importBlock.push( `import ${defaultImport ? `${defaultImport.local},${_}` : ''}{${_}${importedNames @@ -57,7 +58,7 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { return `${specifier.imported} as ${specifier.local}`; } }) - .join(`,${_}`)}${_}}${_}from${_}'${id}';` + .join(`,${_}`)}${_}}${_}from${_}'${escapeId(id)}';` ); } } @@ -75,14 +76,14 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { } } if (starExport) { - importBlock.push(`export${_}*${_}from${_}'${id}';`); + importBlock.push(`export${_}*${_}from${_}'${escapeId(id)}';`); } if (namespaceReexports.length > 0) { if ( !imports || !imports.some(specifier => specifier.imported === '*' && specifier.local === name) ) { - importBlock.push(`import${_}*${_}as ${name} from${_}'${id}';`); + importBlock.push(`import${_}*${_}as ${name} from${_}'${escapeId(id)}';`); } for (const specifier of namespaceReexports) { importBlock.push( @@ -102,7 +103,7 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { return `${specifier.imported} as ${specifier.reexported}`; } }) - .join(`,${_}`)}${_}}${_}from${_}'${id}';` + .join(`,${_}`)}${_}}${_}from${_}'${escapeId(id)}';` ); } } diff --git a/src/finalisers/shared/escapeId.ts b/src/finalisers/shared/escapeId.ts new file mode 100644 index 00000000000..53456aeb241 --- /dev/null +++ b/src/finalisers/shared/escapeId.ts @@ -0,0 +1,4 @@ +const quoteRegEx = /'/g; +export function escapeId(id: string) { + return id.replace(quoteRegEx, "\\'"); +} diff --git a/src/finalisers/system.ts b/src/finalisers/system.ts index 0a0653cdb2d..4ee7bb686f9 100644 --- a/src/finalisers/system.ts +++ b/src/finalisers/system.ts @@ -3,6 +3,7 @@ import { ChunkExports, ModuleDeclarations } from '../Chunk'; import { NormalizedOutputOptions } from '../rollup/types'; import { MISSING_EXPORT_SHIM_VARIABLE } from '../utils/variableNames'; import { FinaliserOptions } from './index'; +import { escapeId } from './shared/escapeId'; function getStarExcludes({ dependencies, exports }: ModuleDeclarations): Set { const starExcludes = new Set(exports.map(expt => expt.exported)); @@ -106,7 +107,7 @@ export default function system( const n = options.compact ? '' : '\n'; const _ = options.compact ? '' : ' '; - const dependencyIds = dependencies.map(m => `'${m.id}'`); + const dependencyIds = dependencies.map(m => `'${escapeId(m.id)}'`); const importBindings: string[] = []; let starExcludes: Set | undefined; diff --git a/test/form/samples/quote-id/_config.js b/test/form/samples/quote-id/_config.js new file mode 100644 index 00000000000..e692a93093a --- /dev/null +++ b/test/form/samples/quote-id/_config.js @@ -0,0 +1,19 @@ +module.exports = { + description: 'supports quote characters in external ids', + options: { + output: { + name: 'Q', + globals: { + "quoted'external": 'quotedExternal' + } + }, + plugins: [ + { + resolveId(id, parent) { + if (!parent) return id; + return { id: "quoted'external", external: true }; + } + } + ] + } +}; diff --git a/test/form/samples/quote-id/_expected/amd.js b/test/form/samples/quote-id/_expected/amd.js new file mode 100644 index 00000000000..d782b2c6769 --- /dev/null +++ b/test/form/samples/quote-id/_expected/amd.js @@ -0,0 +1,16 @@ +define(['exports', 'quoted\'external'], function (exports, quoted_external) { 'use strict'; + + + + Object.keys(quoted_external).forEach(function (k) { + if (k !== 'default') Object.defineProperty(exports, k, { + enumerable: true, + get: function () { + return quoted_external[k]; + } + }); + }); + + Object.defineProperty(exports, '__esModule', { value: true }); + +}); diff --git a/test/form/samples/quote-id/_expected/cjs.js b/test/form/samples/quote-id/_expected/cjs.js new file mode 100644 index 00000000000..cb7d18f35df --- /dev/null +++ b/test/form/samples/quote-id/_expected/cjs.js @@ -0,0 +1,16 @@ +'use strict'; + +Object.defineProperty(exports, '__esModule', { value: true }); + +var quoted_external = require('quoted\'external'); + + + +Object.keys(quoted_external).forEach(function (k) { + if (k !== 'default') Object.defineProperty(exports, k, { + enumerable: true, + get: function () { + return quoted_external[k]; + } + }); +}); diff --git a/test/form/samples/quote-id/_expected/es.js b/test/form/samples/quote-id/_expected/es.js new file mode 100644 index 00000000000..644c1ec992f --- /dev/null +++ b/test/form/samples/quote-id/_expected/es.js @@ -0,0 +1 @@ +export * from 'quoted\'external'; diff --git a/test/form/samples/quote-id/_expected/iife.js b/test/form/samples/quote-id/_expected/iife.js new file mode 100644 index 00000000000..9ae63025695 --- /dev/null +++ b/test/form/samples/quote-id/_expected/iife.js @@ -0,0 +1,17 @@ +var Q = (function (exports, quoted_external) { + 'use strict'; + + + + Object.keys(quoted_external).forEach(function (k) { + if (k !== 'default') Object.defineProperty(exports, k, { + enumerable: true, + get: function () { + return quoted_external[k]; + } + }); + }); + + return exports; + +}({}, quotedExternal)); diff --git a/test/form/samples/quote-id/_expected/system.js b/test/form/samples/quote-id/_expected/system.js new file mode 100644 index 00000000000..54c58b65efd --- /dev/null +++ b/test/form/samples/quote-id/_expected/system.js @@ -0,0 +1,18 @@ +System.register('Q', ['quoted\'external'], function (exports) { + 'use strict'; + var _starExcludes = { default: 1 }; + return { + setters: [function (module) { + var _setter = {}; + for (var _$p in module) { + if (!_starExcludes[_$p]) _setter[_$p] = module[_$p]; + } + exports(_setter); + }], + execute: function () { + + + + } + }; +}); diff --git a/test/form/samples/quote-id/_expected/umd.js b/test/form/samples/quote-id/_expected/umd.js new file mode 100644 index 00000000000..a812da3f79c --- /dev/null +++ b/test/form/samples/quote-id/_expected/umd.js @@ -0,0 +1,18 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('quoted'external')) : + typeof define === 'function' && define.amd ? define(['exports', 'quoted'external'], factory) : + (global = global || self, factory(global.Q = {}, global.quotedExternal)); +}(this, (function (exports, quoted_external) { 'use strict'; + + Object.keys(quoted_external).forEach(function (k) { + if (k !== 'default') Object.defineProperty(exports, k, { + enumerable: true, + get: function () { + return quoted_external[k]; + } + }); + }); + + Object.defineProperty(exports, '__esModule', { value: true }); + +}))); diff --git a/test/form/samples/quote-id/main.js b/test/form/samples/quote-id/main.js new file mode 100644 index 00000000000..4953eb6e351 --- /dev/null +++ b/test/form/samples/quote-id/main.js @@ -0,0 +1 @@ +export * from 'external'; From 923fa7cb9ed8d651684d382c65099ad8d6d19524 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 13 Jun 2020 00:44:19 -0700 Subject: [PATCH 2/7] refactor --- src/Chunk.ts | 5 +++-- src/finalisers/amd.ts | 3 +-- src/finalisers/cjs.ts | 10 ++++------ src/finalisers/es.ts | 15 +++++++-------- src/finalisers/system.ts | 3 +-- src/{finalisers/shared => utils}/escapeId.ts | 0 6 files changed, 16 insertions(+), 20 deletions(-) rename src/{finalisers/shared => utils}/escapeId.ts (100%) diff --git a/src/Chunk.ts b/src/Chunk.ts index 9d2fbad3b41..18f06b628a2 100644 --- a/src/Chunk.ts +++ b/src/Chunk.ts @@ -28,6 +28,7 @@ import { collapseSourcemaps } from './utils/collapseSourcemaps'; import { createHash } from './utils/crypto'; import { deconflictChunk } from './utils/deconflictChunk'; import { errFailedValidation, error } from './utils/error'; +import { escapeId } from './utils/escapeId'; import { sortByExecutionOrder } from './utils/executionOrder'; import { assignExportsToMangledNames, assignExportsToNames } from './utils/exportNames'; import getExportMode from './utils/getExportMode'; @@ -633,7 +634,7 @@ export default class Chunk { const depId = dependency instanceof ExternalModule ? renderedDependency.id : dependency.id!; if (dependency instanceof Chunk) renderedDependency.namedExportsMode = dependency.exportMode !== 'default'; - renderedDependency.id = this.getRelativePath(depId, false); + renderedDependency.id = escapeId(this.getRelativePath(depId, false)); } this.finaliseDynamicImports(options); @@ -938,7 +939,7 @@ export default class Chunk { let id: string = undefined as any; let globalName: string = undefined as any; if (dep instanceof ExternalModule) { - id = dep.renderPath; + id = escapeId(dep.renderPath); if (options.format === 'umd' || options.format === 'iife') { globalName = getGlobalName( dep, diff --git a/src/finalisers/amd.ts b/src/finalisers/amd.ts index b313374385e..1f930e8324c 100644 --- a/src/finalisers/amd.ts +++ b/src/finalisers/amd.ts @@ -2,7 +2,6 @@ import { Bundle as MagicStringBundle } from 'magic-string'; import { NormalizedOutputOptions } from '../rollup/types'; import { INTEROP_NAMESPACE_VARIABLE } from '../utils/variableNames'; import { FinaliserOptions } from './index'; -import { escapeId } from './shared/escapeId'; import { compactEsModuleExport, esModuleExport } from './shared/esModuleExport'; import getExportBlock from './shared/getExportBlock'; import getInteropBlock from './shared/getInteropBlock'; @@ -38,7 +37,7 @@ export default function amd( ) { warnOnBuiltins(warn, dependencies); - const deps = dependencies.map(m => `'${removeExtensionFromRelativeAmdId(escapeId(m.id))}'`); + const deps = dependencies.map(m => `'${removeExtensionFromRelativeAmdId(m.id)}'`); const args = dependencies.map(m => m.name); const n = options.compact ? '' : '\n'; const _ = options.compact ? '' : ' '; diff --git a/src/finalisers/cjs.ts b/src/finalisers/cjs.ts index 05cbad226f2..4539f2bd24d 100644 --- a/src/finalisers/cjs.ts +++ b/src/finalisers/cjs.ts @@ -2,7 +2,6 @@ import { Bundle as MagicStringBundle } from 'magic-string'; import { NormalizedOutputOptions } from '../rollup/types'; import { INTEROP_DEFAULT_VARIABLE, INTEROP_NAMESPACE_VARIABLE } from '../utils/variableNames'; import { FinaliserOptions } from './index'; -import { escapeId } from './shared/escapeId'; import { compactEsModuleExport, esModuleExport } from './shared/esModuleExport'; import getExportBlock from './shared/getExportBlock'; import { getInteropNamespace } from './shared/getInteropNamespace'; @@ -52,22 +51,21 @@ export default function cjs( importBlock += !options.compact || definingVariable ? `;${n}` : ','; } definingVariable = false; - importBlock += `require('${escapeId(id)}')`; + importBlock += `require('${id}')`; } else { importBlock += options.compact && definingVariable ? ',' : `${importBlock ? `;${n}` : ''}${varOrConst} `; definingVariable = true; if (!options.interop || isChunk || !exportsDefault || !namedExportsMode) { - importBlock += `${name}${_}=${_}require('${escapeId(id)}')`; + importBlock += `${name}${_}=${_}require('${id}')`; } else { needsInterop = true; if (exportsNames) - importBlock += `${name}${_}=${_}require('${escapeId(id)}')${ + importBlock += `${name}${_}=${_}require('${id}')${ options.compact ? ',' : `;\n${varOrConst} ` }${name}__default${_}=${_}${INTEROP_DEFAULT_VARIABLE}(${name})`; - else - importBlock += `${name}${_}=${_}${INTEROP_DEFAULT_VARIABLE}(require('${escapeId(id)}'))`; + else importBlock += `${name}${_}=${_}${INTEROP_DEFAULT_VARIABLE}(require('${id}'))`; } } } diff --git a/src/finalisers/es.ts b/src/finalisers/es.ts index b6ffa61a12e..2f3f3977c33 100644 --- a/src/finalisers/es.ts +++ b/src/finalisers/es.ts @@ -2,7 +2,6 @@ import { Bundle as MagicStringBundle } from 'magic-string'; import { ChunkDependencies, ChunkExports, ImportSpecifier, ReexportSpecifier } from '../Chunk'; import { NormalizedOutputOptions } from '../rollup/types'; import { FinaliserOptions } from './index'; -import { escapeId } from './shared/escapeId'; export default function es( magicString: MagicStringBundle, @@ -27,7 +26,7 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { const importBlock: string[] = []; for (const { id, reexports, imports, name } of dependencies) { if (!reexports && !imports) { - importBlock.push(`import${_}'${escapeId(id)}';`); + importBlock.push(`import${_}'${id}';`); continue; } if (imports) { @@ -44,10 +43,10 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { } } if (starImport) { - importBlock.push(`import${_}*${_}as ${starImport.local} from${_}'${escapeId(id)}';`); + importBlock.push(`import${_}*${_}as ${starImport.local} from${_}'${id}';`); } if (defaultImport && importedNames.length === 0) { - importBlock.push(`import ${defaultImport.local} from${_}'${escapeId(id)}';`); + importBlock.push(`import ${defaultImport.local} from${_}'${id}';`); } else if (importedNames.length > 0) { importBlock.push( `import ${defaultImport ? `${defaultImport.local},${_}` : ''}{${_}${importedNames @@ -58,7 +57,7 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { return `${specifier.imported} as ${specifier.local}`; } }) - .join(`,${_}`)}${_}}${_}from${_}'${escapeId(id)}';` + .join(`,${_}`)}${_}}${_}from${_}'${id}';` ); } } @@ -76,14 +75,14 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { } } if (starExport) { - importBlock.push(`export${_}*${_}from${_}'${escapeId(id)}';`); + importBlock.push(`export${_}*${_}from${_}'${id}';`); } if (namespaceReexports.length > 0) { if ( !imports || !imports.some(specifier => specifier.imported === '*' && specifier.local === name) ) { - importBlock.push(`import${_}*${_}as ${name} from${_}'${escapeId(id)}';`); + importBlock.push(`import${_}*${_}as ${name} from${_}'${id}';`); } for (const specifier of namespaceReexports) { importBlock.push( @@ -103,7 +102,7 @@ function getImportBlock(dependencies: ChunkDependencies, _: string): string[] { return `${specifier.imported} as ${specifier.reexported}`; } }) - .join(`,${_}`)}${_}}${_}from${_}'${escapeId(id)}';` + .join(`,${_}`)}${_}}${_}from${_}'${id}';` ); } } diff --git a/src/finalisers/system.ts b/src/finalisers/system.ts index 4ee7bb686f9..0a0653cdb2d 100644 --- a/src/finalisers/system.ts +++ b/src/finalisers/system.ts @@ -3,7 +3,6 @@ import { ChunkExports, ModuleDeclarations } from '../Chunk'; import { NormalizedOutputOptions } from '../rollup/types'; import { MISSING_EXPORT_SHIM_VARIABLE } from '../utils/variableNames'; import { FinaliserOptions } from './index'; -import { escapeId } from './shared/escapeId'; function getStarExcludes({ dependencies, exports }: ModuleDeclarations): Set { const starExcludes = new Set(exports.map(expt => expt.exported)); @@ -107,7 +106,7 @@ export default function system( const n = options.compact ? '' : '\n'; const _ = options.compact ? '' : ' '; - const dependencyIds = dependencies.map(m => `'${escapeId(m.id)}'`); + const dependencyIds = dependencies.map(m => `'${m.id}'`); const importBindings: string[] = []; let starExcludes: Set | undefined; diff --git a/src/finalisers/shared/escapeId.ts b/src/utils/escapeId.ts similarity index 100% rename from src/finalisers/shared/escapeId.ts rename to src/utils/escapeId.ts From 710735ac81dd4b66a4347dc61870b581e7150891 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 13 Jun 2020 00:48:47 -0700 Subject: [PATCH 3/7] match newlines too --- src/utils/escapeId.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/escapeId.ts b/src/utils/escapeId.ts index 53456aeb241..1690c967290 100644 --- a/src/utils/escapeId.ts +++ b/src/utils/escapeId.ts @@ -1,4 +1,4 @@ -const quoteRegEx = /'/g; +const quoteNewlineRegEx = /\'|\r\n?|\n|\u2028|\u2029/g; export function escapeId(id: string) { - return id.replace(quoteRegEx, "\\'"); + return id.replace(quoteNewlineRegEx, "\\'"); } From fdf3f0ef16c98ff8d578a3af6ef55fe9a51efba5 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 13 Jun 2020 00:50:25 -0700 Subject: [PATCH 4/7] fixup capture group --- src/utils/escapeId.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/escapeId.ts b/src/utils/escapeId.ts index 1690c967290..68029206e5a 100644 --- a/src/utils/escapeId.ts +++ b/src/utils/escapeId.ts @@ -1,4 +1,4 @@ -const quoteNewlineRegEx = /\'|\r\n?|\n|\u2028|\u2029/g; +const quoteNewlineRegEx = /(\'|\r\n?|\n|\u2028|\u2029)/g; export function escapeId(id: string) { - return id.replace(quoteNewlineRegEx, "\\'"); + return id.replace(quoteNewlineRegEx, '\\$1'); } From 72798c7b5dd17fb803d8bf0b8a1f53879af0aab6 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 13 Jun 2020 00:54:25 -0700 Subject: [PATCH 5/7] ensure single character replacement --- src/utils/escapeId.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/escapeId.ts b/src/utils/escapeId.ts index 68029206e5a..eb0342003d3 100644 --- a/src/utils/escapeId.ts +++ b/src/utils/escapeId.ts @@ -1,4 +1,4 @@ -const quoteNewlineRegEx = /(\'|\r\n?|\n|\u2028|\u2029)/g; +const quoteNewlineRegEx = /(\'|\r|\n|\u2028|\u2029)/g; export function escapeId(id: string) { return id.replace(quoteNewlineRegEx, '\\$1'); } From 6a0281325db3c6ee17bef0f47861531e72d02476 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 13 Jun 2020 00:58:15 -0700 Subject: [PATCH 6/7] u m d --- src/utils/escapeId.ts | 2 +- test/form/samples/quote-id/_expected/umd.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/escapeId.ts b/src/utils/escapeId.ts index eb0342003d3..a29da6c11b1 100644 --- a/src/utils/escapeId.ts +++ b/src/utils/escapeId.ts @@ -1,4 +1,4 @@ -const quoteNewlineRegEx = /(\'|\r|\n|\u2028|\u2029)/g; +const quoteNewlineRegEx = /(['\r\n\u2028\u2029])/g; export function escapeId(id: string) { return id.replace(quoteNewlineRegEx, '\\$1'); } diff --git a/test/form/samples/quote-id/_expected/umd.js b/test/form/samples/quote-id/_expected/umd.js index a812da3f79c..7bb53852eac 100644 --- a/test/form/samples/quote-id/_expected/umd.js +++ b/test/form/samples/quote-id/_expected/umd.js @@ -1,6 +1,6 @@ (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('quoted'external')) : - typeof define === 'function' && define.amd ? define(['exports', 'quoted'external'], factory) : + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('quoted\'external')) : + typeof define === 'function' && define.amd ? define(['exports', 'quoted\'external'], factory) : (global = global || self, factory(global.Q = {}, global.quotedExternal)); }(this, (function (exports, quoted_external) { 'use strict'; From 54fb204114d67a13809b627b2f4a38a338ee8bec Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sat, 13 Jun 2020 21:15:49 +0200 Subject: [PATCH 7/7] Extend test to cover renormalized paths and line-breaks and fix line-break logic --- src/Chunk.ts | 35 +++++++++---------- src/utils/escapeId.ts | 12 +++++-- test/form/samples/quote-id/_config.js | 16 +++++++-- test/form/samples/quote-id/_expected/amd.js | 15 ++------ test/form/samples/quote-id/_expected/cjs.js | 16 ++------- test/form/samples/quote-id/_expected/es.js | 5 ++- test/form/samples/quote-id/_expected/iife.js | 17 ++------- .../form/samples/quote-id/_expected/system.js | 14 ++++---- test/form/samples/quote-id/_expected/umd.js | 19 +++------- test/form/samples/quote-id/main.js | 4 ++- 10 files changed, 65 insertions(+), 88 deletions(-) diff --git a/src/Chunk.ts b/src/Chunk.ts index 18f06b628a2..429db2079f9 100644 --- a/src/Chunk.ts +++ b/src/Chunk.ts @@ -629,12 +629,16 @@ export default class Chunk { // populate ids in the rendered declarations only here // as chunk ids known only after prerender for (const dependency of this.dependencies) { - if (dependency instanceof ExternalModule && !dependency.renormalizeRenderPath) continue; const renderedDependency = this.renderedDependencies!.get(dependency)!; - const depId = dependency instanceof ExternalModule ? renderedDependency.id : dependency.id!; - if (dependency instanceof Chunk) + if (dependency instanceof ExternalModule) { + const originalId = dependency.renderPath; + renderedDependency.id = escapeId( + dependency.renormalizeRenderPath ? this.getRelativePath(originalId, false) : originalId + ); + } else { renderedDependency.namedExportsMode = dependency.exportMode !== 'default'; - renderedDependency.id = escapeId(this.getRelativePath(depId, false)); + renderedDependency.id = escapeId(this.getRelativePath(dependency.id!, false)); + } } this.finaliseDynamicImports(options); @@ -936,25 +940,18 @@ export default class Chunk { namedExportsMode = dep.exportMode !== 'default'; } - let id: string = undefined as any; - let globalName: string = undefined as any; - if (dep instanceof ExternalModule) { - id = escapeId(dep.renderPath); - if (options.format === 'umd' || options.format === 'iife') { - globalName = getGlobalName( + dependencies.set(dep, { + exportsDefault, + exportsNames, + globalName: (dep instanceof ExternalModule && + (options.format === 'umd' || options.format === 'iife') && + getGlobalName( dep, options.globals, exportsNames || exportsDefault, this.inputOptions.onwarn - )!; - } - } - - dependencies.set(dep, { - exportsDefault, - exportsNames, - globalName, - id, // chunk id updated on render + )) as string, + id: undefined as any, // chunk id updated on render imports: imports.length > 0 ? imports : (null as any), isChunk: dep instanceof Chunk, name: dep.variableName, diff --git a/src/utils/escapeId.ts b/src/utils/escapeId.ts index a29da6c11b1..da413d41c95 100644 --- a/src/utils/escapeId.ts +++ b/src/utils/escapeId.ts @@ -1,4 +1,12 @@ -const quoteNewlineRegEx = /(['\r\n\u2028\u2029])/g; +const quoteNewlineRegEx = /['\r\n\u2028\u2029]/g; +const replacements = { + '\n': '\\n', + '\r': '\\r', + "'": "\\'", + '\u2028': '\\u2028', + '\u2029': '\\u2029' +}; + export function escapeId(id: string) { - return id.replace(quoteNewlineRegEx, '\\$1'); + return id.replace(quoteNewlineRegEx, match => replacements[match as keyof typeof replacements]); } diff --git a/test/form/samples/quote-id/_config.js b/test/form/samples/quote-id/_config.js index e692a93093a..3dfa80c9996 100644 --- a/test/form/samples/quote-id/_config.js +++ b/test/form/samples/quote-id/_config.js @@ -1,17 +1,27 @@ +const path = require('path'); + +const external1 = "quoted'\r\n\u2028\u2029external1"; +const external2 = path.join(__dirname, "quoted'\r\n\u2028\u2029external2"); + module.exports = { description: 'supports quote characters in external ids', options: { output: { name: 'Q', globals: { - "quoted'external": 'quotedExternal' + [external1]: 'quotedExternal1', + [external2]: 'quotedExternal2' } }, plugins: [ { resolveId(id, parent) { - if (!parent) return id; - return { id: "quoted'external", external: true }; + if (id === 'external1') { + return { id: external1, external: true }; + } + if (id === 'external2') { + return { id: external2, external: true }; + } } } ] diff --git a/test/form/samples/quote-id/_expected/amd.js b/test/form/samples/quote-id/_expected/amd.js index d782b2c6769..b908f89dd23 100644 --- a/test/form/samples/quote-id/_expected/amd.js +++ b/test/form/samples/quote-id/_expected/amd.js @@ -1,16 +1,5 @@ -define(['exports', 'quoted\'external'], function (exports, quoted_external) { 'use strict'; +define(['quoted\'\r\n\u2028\u2029external1', './quoted\'\r\n\u2028\u2029external2'], function (quoted_____external1, quoted_____external2) { 'use strict'; - - - Object.keys(quoted_external).forEach(function (k) { - if (k !== 'default') Object.defineProperty(exports, k, { - enumerable: true, - get: function () { - return quoted_external[k]; - } - }); - }); - - Object.defineProperty(exports, '__esModule', { value: true }); + console.log(quoted_____external1.foo, quoted_____external2.bar); }); diff --git a/test/form/samples/quote-id/_expected/cjs.js b/test/form/samples/quote-id/_expected/cjs.js index cb7d18f35df..28ce92f286d 100644 --- a/test/form/samples/quote-id/_expected/cjs.js +++ b/test/form/samples/quote-id/_expected/cjs.js @@ -1,16 +1,6 @@ 'use strict'; -Object.defineProperty(exports, '__esModule', { value: true }); +var quoted_____external1 = require('quoted\'\r\n\u2028\u2029external1'); +var quoted_____external2 = require('./quoted\'\r\n\u2028\u2029external2'); -var quoted_external = require('quoted\'external'); - - - -Object.keys(quoted_external).forEach(function (k) { - if (k !== 'default') Object.defineProperty(exports, k, { - enumerable: true, - get: function () { - return quoted_external[k]; - } - }); -}); +console.log(quoted_____external1.foo, quoted_____external2.bar); diff --git a/test/form/samples/quote-id/_expected/es.js b/test/form/samples/quote-id/_expected/es.js index 644c1ec992f..7d54a160fef 100644 --- a/test/form/samples/quote-id/_expected/es.js +++ b/test/form/samples/quote-id/_expected/es.js @@ -1 +1,4 @@ -export * from 'quoted\'external'; +import { foo } from 'quoted\'\r\n\u2028\u2029external1'; +import { bar } from './quoted\'\r\n\u2028\u2029external2'; + +console.log(foo, bar); diff --git a/test/form/samples/quote-id/_expected/iife.js b/test/form/samples/quote-id/_expected/iife.js index 9ae63025695..2f853d9bceb 100644 --- a/test/form/samples/quote-id/_expected/iife.js +++ b/test/form/samples/quote-id/_expected/iife.js @@ -1,17 +1,6 @@ -var Q = (function (exports, quoted_external) { +(function (quoted_____external1, quoted_____external2) { 'use strict'; + console.log(quoted_____external1.foo, quoted_____external2.bar); - - Object.keys(quoted_external).forEach(function (k) { - if (k !== 'default') Object.defineProperty(exports, k, { - enumerable: true, - get: function () { - return quoted_external[k]; - } - }); - }); - - return exports; - -}({}, quotedExternal)); +}(quotedExternal1, quotedExternal2)); diff --git a/test/form/samples/quote-id/_expected/system.js b/test/form/samples/quote-id/_expected/system.js index 54c58b65efd..e6face3d7c8 100644 --- a/test/form/samples/quote-id/_expected/system.js +++ b/test/form/samples/quote-id/_expected/system.js @@ -1,17 +1,15 @@ -System.register('Q', ['quoted\'external'], function (exports) { +System.register('Q', ['quoted\'\r\n\u2028\u2029external1', './quoted\'\r\n\u2028\u2029external2'], function () { 'use strict'; - var _starExcludes = { default: 1 }; + var foo, bar; return { setters: [function (module) { - var _setter = {}; - for (var _$p in module) { - if (!_starExcludes[_$p]) _setter[_$p] = module[_$p]; - } - exports(_setter); + foo = module.foo; + }, function (module) { + bar = module.bar; }], execute: function () { - + console.log(foo, bar); } }; diff --git a/test/form/samples/quote-id/_expected/umd.js b/test/form/samples/quote-id/_expected/umd.js index 7bb53852eac..605d5b089b1 100644 --- a/test/form/samples/quote-id/_expected/umd.js +++ b/test/form/samples/quote-id/_expected/umd.js @@ -1,18 +1,9 @@ (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('quoted\'external')) : - typeof define === 'function' && define.amd ? define(['exports', 'quoted\'external'], factory) : - (global = global || self, factory(global.Q = {}, global.quotedExternal)); -}(this, (function (exports, quoted_external) { 'use strict'; + typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('quoted\'\r\n\u2028\u2029external1'), require('./quoted\'\r\n\u2028\u2029external2')) : + typeof define === 'function' && define.amd ? define(['quoted\'\r\n\u2028\u2029external1', './quoted\'\r\n\u2028\u2029external2'], factory) : + (global = global || self, factory(global.quotedExternal1, global.quotedExternal2)); +}(this, (function (quoted_____external1, quoted_____external2) { 'use strict'; - Object.keys(quoted_external).forEach(function (k) { - if (k !== 'default') Object.defineProperty(exports, k, { - enumerable: true, - get: function () { - return quoted_external[k]; - } - }); - }); - - Object.defineProperty(exports, '__esModule', { value: true }); + console.log(quoted_____external1.foo, quoted_____external2.bar); }))); diff --git a/test/form/samples/quote-id/main.js b/test/form/samples/quote-id/main.js index 4953eb6e351..f392585109d 100644 --- a/test/form/samples/quote-id/main.js +++ b/test/form/samples/quote-id/main.js @@ -1 +1,3 @@ -export * from 'external'; +import { foo } from 'external1'; +import { bar } from 'external2'; +console.log(foo, bar);