Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fully support arbitrary module namespace identifiers for all formats #5298

Merged
merged 2 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion src/Module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ export default class Module {
if (node.exported) {
// export * as name from './other'

const name = node.exported.name;
const name = node.exported instanceof Literal ? node.exported.value : node.exported.name;
this.assertUniqueExportName(name, node.exported.start);
this.reexportDescriptions.set(name, {
localName: '*',
Expand Down
2 changes: 1 addition & 1 deletion src/ast/nodes/ExportAllDeclaration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { NodeBase } from './shared/Node';

export default class ExportAllDeclaration extends NodeBase {
declare attributes: ImportAttribute[];
declare exported: Identifier | null;
declare exported: Identifier | Literal<string> | null;
declare needsBoundaries: true;
declare source: Literal<string>;
declare type: NodeType.tExportAllDeclaration;
Expand Down
5 changes: 4 additions & 1 deletion src/ast/variables/NamespaceVariable.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { AstContext, default as Module } from '../../Module';
import { stringifyObjectKeyIfNeeded } from '../../utils/identifierHelpers';
import { getToStringTagValue, MERGE_NAMESPACES_VARIABLE } from '../../utils/interopHelpers';
import type { RenderOptions } from '../../utils/renderHelpers';
import { getSystemExportStatement } from '../../utils/systemJsRendering';
Expand Down Expand Up @@ -139,7 +140,9 @@ export default class NamespaceVariable extends Variable {
if (this.referencedEarly || variable.isReassigned || variable === this) {
return [
null,
`get ${name}${_}()${_}{${_}return ${variable.getName(getPropertyAccess)}${s}${_}}`
`get ${stringifyObjectKeyIfNeeded(name)}${_}()${_}{${_}return ${variable.getName(
getPropertyAccess
)}${s}${_}}`
];
}

Expand Down
15 changes: 10 additions & 5 deletions src/finalisers/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { Bundle as MagicStringBundle } from 'magic-string';
import type { ChunkDependency, ChunkExports, ImportSpecifier, ReexportSpecifier } from '../Chunk';
import type { NormalizedOutputOptions } from '../rollup/types';
import type { GenerateCodeSnippets } from '../utils/generateCodeSnippets';
import { stringifyIdentifierIfNeeded } from '../utils/identifierHelpers';
import { getHelpersBlock } from '../utils/interopHelpers';
import type { FinaliserOptions } from './index';

Expand Down Expand Up @@ -68,7 +69,7 @@ function getImportBlock(
.map(specifier =>
specifier.imported === specifier.local
? specifier.imported
: `${specifier.imported} as ${specifier.local}`
: `${stringifyIdentifierIfNeeded(specifier.imported)} as ${specifier.local}`
)
.join(`,${_}`)}${_}}${_}from${_}${pathWithAssertion}`
);
Expand Down Expand Up @@ -100,7 +101,9 @@ function getImportBlock(
for (const specifier of namespaceReexports) {
importBlock.push(
`export${_}{${_}${
name === specifier.reexported ? name : `${name} as ${specifier.reexported}`
name === specifier.reexported
? name
: `${name} as ${stringifyIdentifierIfNeeded(specifier.reexported)}`
} };`
);
}
Expand All @@ -110,8 +113,10 @@ function getImportBlock(
`export${_}{${_}${namedReexports
.map(specifier =>
specifier.imported === specifier.reexported
? specifier.imported
: `${specifier.imported} as ${specifier.reexported}`
? stringifyIdentifierIfNeeded(specifier.imported)
: `${stringifyIdentifierIfNeeded(
specifier.imported
)} as ${stringifyIdentifierIfNeeded(specifier.reexported)}`
)
.join(`,${_}`)}${_}}${_}from${_}${pathWithAssertion}`
);
Expand All @@ -131,7 +136,7 @@ function getExportBlock(exports: ChunkExports, { _, cnst }: GenerateCodeSnippets
exportDeclaration.push(
specifier.exported === specifier.local
? specifier.local
: `${specifier.local} as ${specifier.exported}`
: `${specifier.local} as ${stringifyIdentifierIfNeeded(specifier.exported)}`
);
}
if (exportDeclaration.length > 0) {
Expand Down
4 changes: 3 additions & 1 deletion src/finalisers/shared/getExportBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ export function getExportBlock(
name: null
});
exportBlock +=
`Object.defineProperty(exports,${_}'${specifier.reexported}',${_}{${n}` +
`Object.defineProperty(exports,${_}${JSON.stringify(
specifier.reexported
)},${_}{${n}` +
`${t}enumerable:${_}true,${n}` +
`${t}get:${_}${left}${importName}${right}${n}});`;
} else {
Expand Down
6 changes: 5 additions & 1 deletion src/utils/deconflictChunk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import ExportDefaultVariable from '../ast/variables/ExportDefaultVariable';
import type SyntheticNamedExportVariable from '../ast/variables/SyntheticNamedExportVariable';
import type Variable from '../ast/variables/Variable';
import type { GetInterop, InternalModuleFormat } from '../rollup/types';
import { makeLegal } from './identifierHelpers';
import {
canDefaultBeTakenFromNamespace,
defaultInteropHelpersByInteropType,
Expand Down Expand Up @@ -127,7 +128,10 @@ function deconflictImportsEsmOrSystem(
)
);
} else {
variable.setRenderNames(null, getSafeName(name, usedNames, variable.forbiddenNames));
variable.setRenderNames(
null,
getSafeName(makeLegal(name), usedNames, variable.forbiddenNames)
);
}
}
for (const variable of syntheticExports) {
Expand Down
13 changes: 6 additions & 7 deletions src/utils/generateCodeSnippets.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { NormalizedOutputOptions } from '../rollup/types';
import RESERVED_NAMES from './RESERVED_NAMES';
import { stringifyObjectKeyIfNeeded, VALID_IDENTIFIER_REGEXP } from './identifierHelpers';

export interface GenerateCodeSnippets {
_: string;
Expand Down Expand Up @@ -83,8 +84,8 @@ export function getGenerateCodeSnippets({
];

const isValidPropertyName = reservedNamesAsProps
? (name: string): boolean => validPropertyName.test(name)
: (name: string): boolean => !RESERVED_NAMES.has(name) && validPropertyName.test(name);
? (name: string): boolean => VALID_IDENTIFIER_REGEXP.test(name)
: (name: string): boolean => !RESERVED_NAMES.has(name) && VALID_IDENTIFIER_REGEXP.test(name);

return {
_,
Expand Down Expand Up @@ -112,10 +113,10 @@ export function getGenerateCodeSnippets({
return `{${fields
.map(([key, value]) => {
if (key === null) return `${prefix}${value}`;
const needsQuotes = !isValidPropertyName(key);
return key === value && objectShorthand && !needsQuotes
const keyInObject = stringifyObjectKeyIfNeeded(key);
return key === value && objectShorthand && key === keyInObject
? prefix + key
: `${prefix}${needsQuotes ? `'${key}'` : key}:${_}${value}`;
: `${prefix}${keyInObject}:${_}${value}`;
})
.join(`,`)}${
fields.length === 0 ? '' : lineBreakIndent ? `${n}${lineBreakIndent.base}` : _
Expand All @@ -130,5 +131,3 @@ export function getGenerateCodeSnippets({

const wrapIfNeeded = (code: string, needsParens: boolean | undefined): string =>
needsParens ? `(${code})` : code;

const validPropertyName = /^(?!\d)[\w$]+$/;
17 changes: 17 additions & 0 deletions src/utils/identifierHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,20 @@ export function makeLegal(value: string): string {

return value || '_';
}

export const VALID_IDENTIFIER_REGEXP = /^[$_\p{ID_Start}][$\u200C\u200D\p{ID_Continue}]*$/u;
export const NUMBER_REGEXP = /^\d+$/;

export function stringifyObjectKeyIfNeeded(key: string) {
if (VALID_IDENTIFIER_REGEXP.test(key) || NUMBER_REGEXP.test(key)) {
return key;
}
return JSON.stringify(key);
}

export function stringifyIdentifierIfNeeded(key: string) {
if (VALID_IDENTIFIER_REGEXP.test(key)) {
return key;
}
return JSON.stringify(key);
}
9 changes: 5 additions & 4 deletions src/utils/systemJsRendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ export function getSystemExportStatement(
exportNamesByVariable.get(exportedVariables[0])!.length === 1
) {
const variable = exportedVariables[0];
return `exports('${exportNamesByVariable.get(variable)}',${_}${variable.getName(
getPropertyAccess
)}${modifier})`;
return `exports(${JSON.stringify(
exportNamesByVariable.get(variable)![0]
)},${_}${variable.getName(getPropertyAccess)}${modifier})`;
} else {
const fields: [key: string, value: string][] = [];
for (const variable of exportedVariables) {
Expand All @@ -26,6 +26,7 @@ export function getSystemExportStatement(
}
}

// This is only invoked if there is exactly one export name
export function renderSystemExportExpression(
exportedVariable: Variable,
expressionStart: number,
Expand All @@ -35,7 +36,7 @@ export function renderSystemExportExpression(
): void {
code.prependRight(
expressionStart,
`exports('${exportNamesByVariable.get(exportedVariable)}',${_}`
`exports(${JSON.stringify(exportNamesByVariable.get(exportedVariable)![0])},${_}`
);
code.appendLeft(expressionEnd, ')');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

const something = exports('something', 42);
const something = exports("something", 42);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

const something = exports('something', 42);
const something = exports("something", 42);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

const something = exports('something', 42);
const something = exports("something", 42);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ System.register(['./generated-dep2.js'], (function (exports) {
fn();
fn$1();
}
} exports('default', Main1);
} exports("default", Main1);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ System.register(['./generated-dep2.js'], (function (exports) {
fn();
fn$2();
}
} exports('default', Main2);
} exports("default", Main2);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

var c = exports('c', {});
var c = exports("c", {});

(function (exports) {
exports.preFaPrint = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

var x = exports('x', 42);
var x = exports("x", 42);
console.log('dep1');

})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

var x = exports('x', 43);
var x = exports("x", 43);
console.log('dep2');

})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

const x = exports('x', 0);
const x = exports("x", 0);
console.log('112');

})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ System.register(['./generated-dep1.js'], (function (exports) {
constructor () {
fn();
}
} exports('default', Main1);
} exports("default", Main1);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ System.register(['./generated-dep1.js'], (function (exports) {
constructor () {
fn();
}
} exports('default', Main2);
} exports("default", Main2);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ System.register([], (function (exports) {
test() {
return ONE_CONSTANT;
}
} exports('a', One);
} exports("a", One);

const ONE_CONSTANT = exports('O', 'oneconstant');
const ONE_CONSTANT = exports("O", 'oneconstant');

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ System.register(['./generated-main1.js'], (function (exports) {
test() {
return ONE_CONSTANT;
}
} exports('ItemTwo', Two);
} exports("ItemTwo", Two);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ System.register([], (function (exports) {
function fn$1 () {
fn$2();
console.log(text);
exports('b', text$1 = 'dep1 fn after dep2');
exports("b", text$1 = 'dep1 fn after dep2');
}

var text$1 = exports('b', 'dep1 fn');
var text$1 = exports("b", 'dep1 fn');

function fn () {
console.log(text$1);
exports('t', text = 'dep2 fn after dep1');
exports("t", text = 'dep2 fn after dep1');
}

var text = exports('t', 'dep2 fn');
var text = exports("t", 'dep2 fn');

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ System.register(['./generated-dep1.js'], (function (exports) {
fn();
console.log(text);
}
} exports('default', Main1);
} exports("default", Main1);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ System.register(['./generated-dep1.js'], (function (exports) {
fn();
console.log(text);
}
} exports('default', Main2);
} exports("default", Main2);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ System.register([], (function (exports) {
return {
execute: (function () {

var commonjsGlobal = exports('c', typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {});
var commonjsGlobal = exports("c", typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {});

commonjsGlobal.data = [4, 5, 6];
var shared = exports('s', commonjsGlobal.data);
var shared = exports("s", commonjsGlobal.data);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

var num = exports('n', 1);
var num = exports("n", 1);

})
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ System.register([], (function (exports) {
return {
execute: (function () {

var num = exports('n', 2);
var num = exports("n", 2);

})
};
Expand Down