Skip to content

Commit

Permalink
Make namespace @@toStringTag "Module" non-enumerable (#4378)
Browse files Browse the repository at this point in the history 
* test: namespace export should have @@toStringTag with correct property descriptors

* add additional Object.assign and object spread test cases

* add more asserts

* use deepStrictEqual

* Make toStringTag non-enumerable for dynamic namespaces

* Make toStringTag non-enumerable for interop namespaces

* Do not freeze merged namespaces twice

* Generate toStringTag in merged namespaces

* Simplify output by defining properties together

* Deprecate namespaceToStringTag in favour of generatedCode.symbols

* Deprecate namespaceToStringTag in favour of generatedCode.symbols

Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>
  • Loading branch information
@dnalborczyk @lukastaegert
3 people committed Mar 2, 2022
1 parent 11bd9d9 commit b74cb92
Show file tree
Hide file tree
Showing 222 changed files with 1,369 additions and 298 deletions.
64 changes: 46 additions & 18 deletions docs/999-big-list-of-options.md
View file
Expand Up @@ -460,7 +460,7 @@ Whether to extend the global variable defined by the `name` option in `umd` or `

#### output.generatedCode

Type: `"es5" | "es2015" | { arrowFunctions?: boolean, constBindings?: boolean, objectShorthand?: boolean, preset?: "es5" | "es2015", reservedNamesAsProps?: boolean }`<br> CLI: `--generatedCode <preset>`<br> Default: `"es5"`
Type: `"es5" | "es2015" | { arrowFunctions?: boolean, constBindings?: boolean, objectShorthand?: boolean, preset?: "es5" | "es2015", reservedNamesAsProps?: boolean, symbols?: boolean }`<br> CLI: `--generatedCode <preset>`<br> Default: `"es5"`

Which language features Rollup can safely use in generated code. This will not transpile any user code but only change the code Rollup uses in wrappers and helpers. You may choose one of several presets:

Expand Down Expand Up @@ -577,6 +577,34 @@ const foo = null;
exports.void = foo;
```

**output.generatedCode.symbols**<br> Type: `boolean`<br> CLI: `--generatedCode.symbols`/`--no-generatedCode.symbols`<br> Default: `false`

Whether to allow the use of `Symbol` in auto-generated code snippets. Currently, this only controls if namespaces will have the `Symbol.toStringTag` property set to the correct value of `Module`, which means that for a namespace, `String(namespace)` logs `[object Module]`. This again is used for feature detection in certain libraries and frameworks.

```javascript
// input
export const foo = 42;

// cjs output with symbols: false
Object.defineProperty(exports, '__esModule', { value: true });

const foo = 42;

exports.foo = foo;

// cjs output with symbols: true
Object.defineProperties(exports, {
__esModule: { value: true },
[Symbol.toStringTag]: { value: 'Module' }
});

const foo = 42;

exports.foo = foo;
```

Note: The `__esModule` flag in the example can be prevented via the [`output.esModule`](https://rollupjs.org/guide/en/#outputesmodule) option.

#### output.hoistTransitiveImports

Type: `boolean`<br> CLI: `--hoistTransitiveImports`/`--no-hoistTransitiveImports`<br> Default: `true`
Expand Down Expand Up @@ -1417,19 +1445,6 @@ export default {
};
```

#### output.namespaceToStringTag

Type: `boolean`<br> CLI: `--namespaceToStringTag`/`--no-namespaceToStringTag`<br> Default: `false`

Whether to add spec compliant `.toString()` tags to namespace objects. If this option is set,

```javascript
import * as namespace from './file.js';
console.log(String(namespace));
```

will always log `[object Module]`;

#### output.noConflict

Type: `boolean`<br> CLI: `--noConflict`/`--no-noConflict`<br> Default: `false`
Expand Down Expand Up @@ -1848,10 +1863,6 @@ _Use the [`output.inlineDynamicImports`](guide/en/#outputinlinedynamicimports) o

_Use the [`output.manualChunks`](guide/en/#outputmanualchunks) output option instead, which has the same signature._

#### preserveModules

_Use the [`output.preserveModules`](guide/en/#outputpreservemodules) output option instead, which has the same signature._

#### output.dynamicImportFunction

_Use the [`renderDynamicImport`](guide/en/#renderdynamicimport) plugin hook instead._<br> Type: `string`<br> CLI: `--dynamicImportFunction <name>`<br> Default: `import`
Expand Down Expand Up @@ -1890,3 +1901,20 @@ console.log(42);
```

You can also supply a list of external ids to be considered pure or a function that is called whenever an external import could be removed.

#### output.namespaceToStringTag

_Use [`output.generatedCode.symbols`](guide/en/#outputgeneratedcode) instead._<br> Type: `boolean`<br> CLI: `--namespaceToStringTag`/`--no-namespaceToStringTag`<br> Default: `false`

Whether to add spec compliant `.toString()` tags to namespace objects. If this option is set,

```javascript
import * as namespace from './file.js';
console.log(String(namespace));
```

will always log `[object Module]`;

#### preserveModules

_Use the [`output.preserveModules`](guide/en/#outputpreservemodules) output option instead, which has the same signature._
22 changes: 12 additions & 10 deletions src/ast/variables/NamespaceVariable.ts
View file
@@ -1,6 +1,6 @@
import type Module from '../../Module';
import type { AstContext } from '../../Module';
import { MERGE_NAMESPACES_VARIABLE } from '../../utils/interopHelpers';
import { getToStringTagValue, MERGE_NAMESPACES_VARIABLE } from '../../utils/interopHelpers';
import type { RenderOptions } from '../../utils/renderHelpers';
import { getSystemExportStatement } from '../../utils/systemJsRendering';
import type Identifier from '../nodes/Identifier';
Expand Down Expand Up @@ -77,24 +77,26 @@ export default class NamespaceVariable extends Variable {
return [name, original.getName(getPropertyAccess)];
}
);

if (namespaceToStringTag) {
members.unshift([null, `[Symbol.toStringTag]:${_}'Module'`]);
}

members.unshift([null, `__proto__:${_}null`]);

let output = getObject(members, { lineBreakIndent: { base: '', t } });
if (this.mergedNamespaces.length > 0) {
const assignmentArgs = this.mergedNamespaces.map(variable =>
variable.getName(getPropertyAccess)
);
output = `/*#__PURE__*/${MERGE_NAMESPACES_VARIABLE}(${output}, [${assignmentArgs.join(
output = `/*#__PURE__*/${MERGE_NAMESPACES_VARIABLE}(${output},${_}[${assignmentArgs.join(
`,${_}`
)}])`;
}
if (freeze) {
output = `/*#__PURE__*/Object.freeze(${output})`;
} else {
// The helper to merge namespaces will also take care of freezing and toStringTag
if (namespaceToStringTag) {
output = `/*#__PURE__*/Object.defineProperty(${output},${_}Symbol.toStringTag,${_}${getToStringTagValue(
getObject
)})`;
}
if (freeze) {
output = `/*#__PURE__*/Object.freeze(${output})`;
}
}

const name = this.getName(getPropertyAccess);
Expand Down
3 changes: 1 addition & 2 deletions src/finalisers/amd.ts
View file
Expand Up @@ -86,8 +86,7 @@ export default function amd(
namedExportsMode && hasExports,
isEntryFacade && esModule,
isModuleFacade && namespaceToStringTag,
_,
n
snippets
);
if (namespaceMarkers) {
namespaceMarkers = n + n + namespaceMarkers;
Expand Down
3 changes: 1 addition & 2 deletions src/finalisers/cjs.ts
View file
Expand Up @@ -38,8 +38,7 @@ export default function cjs(
namedExportsMode && hasExports,
isEntryFacade && esModule,
isModuleFacade && namespaceToStringTag,
_,
n
snippets
);
if (namespaceMarkers) {
namespaceMarkers += n + n;
Expand Down
3 changes: 1 addition & 2 deletions src/finalisers/iife.ts
View file
Expand Up @@ -122,8 +122,7 @@ export default function iife(
namedExportsMode && hasExports,
esModule,
namespaceToStringTag,
_,
n
snippets
);
if (namespaceMarkers) {
namespaceMarkers = n + n + namespaceMarkers;
Expand Down
37 changes: 22 additions & 15 deletions src/finalisers/shared/getExportBlock.ts
View file
Expand Up @@ -3,6 +3,7 @@ import type { GetInterop } from '../../rollup/types';
import type { GenerateCodeSnippets } from '../../utils/generateCodeSnippets';
import {
defaultInteropHelpersByInteropType,
getToStringTagValue,
isDefaultAProperty,
namespaceInteropHelpersByInteropType
} from '../../utils/interopHelpers';
Expand Down Expand Up @@ -189,34 +190,40 @@ function getReexportedImportName(
return `${moduleVariableName}${getPropertyAccess(imported)}`;
}

function getEsModuleExport(_: string): string {
return `Object.defineProperty(exports,${_}'__esModule',${_}{${_}value:${_}true${_}});`;
}

function getNamespaceToStringExport(_: string): string {
return `exports[Symbol.toStringTag]${_}=${_}'Module';`;
function getEsModuleValue(getObject: GenerateCodeSnippets['getObject']) {
return getObject([['value', 'true']], {
lineBreakIndent: null
});
}

export function getNamespaceMarkers(
hasNamedExports: boolean,
addEsModule: boolean,
addNamespaceToStringTag: boolean,
_: string,
n: string
{ _, getObject }: GenerateCodeSnippets
): string {
let namespaceMarkers = '';
if (hasNamedExports) {
if (addEsModule) {
namespaceMarkers += getEsModuleExport(_);
if (addNamespaceToStringTag) {
return `Object.defineProperties(exports,${_}${getObject(
[
['__esModule', getEsModuleValue(getObject)],
[null, `[Symbol.toStringTag]:${_}${getToStringTagValue(getObject)}`]
],
{
lineBreakIndent: null
}
)});`;
}
return `Object.defineProperty(exports,${_}'__esModule',${_}${getEsModuleValue(getObject)});`;
}
if (addNamespaceToStringTag) {
if (namespaceMarkers) {
namespaceMarkers += n;
}
namespaceMarkers += getNamespaceToStringExport(_);
return `Object.defineProperty(exports,${_}Symbol.toStringTag,${_}${getToStringTagValue(
getObject
)});`;
}
}
return namespaceMarkers;
return '';
}

const getDefineProperty = (
Expand Down
3 changes: 1 addition & 2 deletions src/finalisers/umd.ts
View file
Expand Up @@ -203,8 +203,7 @@ export default function umd(
namedExportsMode && hasExports,
esModule,
namespaceToStringTag,
_,
n
snippets
);
if (namespaceMarkers) {
namespaceMarkers = n + n + namespaceMarkers;
Expand Down
4 changes: 3 additions & 1 deletion src/rollup/types.d.ts
View file
Expand Up @@ -608,6 +608,7 @@ interface NormalizedGeneratedCodeOptions {
constBindings: boolean;
objectShorthand: boolean;
reservedNamesAsProps: boolean;
symbols: boolean;
}

interface GeneratedCodeOptions extends Partial<NormalizedGeneratedCodeOptions> {
Expand Down Expand Up @@ -681,12 +682,13 @@ export interface OutputOptions {
manualChunks?: ManualChunksOption;
minifyInternalExports?: boolean;
name?: string;
/** @deprecated Use "generatedCode.symbols" instead. */
namespaceToStringTag?: boolean;
noConflict?: boolean;
outro?: string | (() => string | Promise<string>);
paths?: OptionsPaths;
plugins?: (OutputPlugin | null | false | undefined)[];
/** @deprecated Use the "generatedCode.constBindings" instead. */
/** @deprecated Use "generatedCode.constBindings" instead. */
preferConst?: boolean;
preserveModules?: boolean;
preserveModulesRoot?: string;
Expand Down
64 changes: 43 additions & 21 deletions src/utils/interopHelpers.ts
View file
Expand Up @@ -105,28 +105,30 @@ const HELPER_GENERATORS: {
},
[INTEROP_NAMESPACE_DEFAULT_ONLY_VARIABLE](
_t,
{ _, getDirectReturnFunction, getObject, n },
snippets,
_liveBindings: boolean,
freeze: boolean,
namespaceToStringTag: boolean
) {
const { getDirectReturnFunction, getObject, n } = snippets;
const [left, right] = getDirectReturnFunction(['e'], {
functionReturn: true,
lineBreakIndent: null,
name: INTEROP_NAMESPACE_DEFAULT_ONLY_VARIABLE
});
return `${left}${getFrozen(
getObject(
[
['__proto__', 'null'],
...(namespaceToStringTag
? [[null, `[Symbol.toStringTag]:${_}'Module'`] as [null, string]]
: []),
['default', 'e']
],
{ lineBreakIndent: null }
),
freeze
freeze,
getWithToStringTag(
namespaceToStringTag,
getObject(
[
['__proto__', 'null'],
['default', 'e']
],
{ lineBreakIndent: null }
),
snippets
)
)}${right}${n}${n}`;
},
[INTEROP_NAMESPACE_DEFAULT_VARIABLE](t, snippets, liveBindings, freeze, namespaceToStringTag) {
Expand Down Expand Up @@ -161,7 +163,7 @@ const HELPER_GENERATORS: {
`}${n}${n}`
);
},
[MERGE_NAMESPACES_VARIABLE](t, snippets, liveBindings, freeze) {
[MERGE_NAMESPACES_VARIABLE](t, snippets, liveBindings, freeze, namespaceToStringTag) {
const { _, cnst, n } = snippets;
const useForEach = cnst === 'var' && liveBindings;
return (
Expand All @@ -180,7 +182,10 @@ const HELPER_GENERATORS: {
t,
snippets
)}${n}` +
`${t}return ${getFrozen('n', freeze)};${n}` +
`${t}return ${getFrozen(
freeze,
getWithToStringTag(namespaceToStringTag, 'n', snippets)
)};${n}` +
`}${n}${n}`
);
}
Expand All @@ -200,7 +205,7 @@ const createNamespaceObject = (
freeze: boolean,
namespaceToStringTag: boolean
) => {
const { _, cnst, getPropertyAccess, n, s } = snippets;
const { _, cnst, getObject, getPropertyAccess, n, s } = snippets;
const copyProperty =
`{${n}` +
(liveBindings ? copyNonDefaultOwnPropertyLiveBinding : copyPropertyStatic)(
Expand All @@ -210,16 +215,16 @@ const createNamespaceObject = (
) +
`${i}${t}}`;
return (
`${i}${cnst} n${_}=${_}${
`${i}${cnst} n${_}=${_}Object.create(null${
namespaceToStringTag
? `{__proto__:${_}null,${_}[Symbol.toStringTag]:${_}'Module'}`
: 'Object.create(null)'
};${n}` +
? `,${_}{${_}[Symbol.toStringTag]:${_}${getToStringTagValue(getObject)}${_}}`
: ''
});${n}` +
`${i}if${_}(e)${_}{${n}` +
`${i}${t}${loopOverKeys(copyProperty, !liveBindings, snippets)}${n}` +
`${i}}${n}` +
`${i}n${getPropertyAccess('default')}${_}=${_}e;${n}` +
`${i}return ${getFrozen('n', freeze)}${s}${n}`
`${i}return ${getFrozen(freeze, 'n')}${s}${n}`
);
};

Expand Down Expand Up @@ -321,7 +326,24 @@ const copyPropertyLiveBinding = (
const copyPropertyStatic = (_t: string, i: string, { _, n }: GenerateCodeSnippets) =>
`${i}n[k]${_}=${_}e[k];${n}`;

const getFrozen = (fragment: string, freeze: boolean) =>
const getFrozen = (freeze: boolean, fragment: string) =>
freeze ? `Object.freeze(${fragment})` : fragment;

const getWithToStringTag = (
namespaceToStringTag: boolean,
fragment: string,
{ _, getObject }: GenerateCodeSnippets
) =>
namespaceToStringTag
? `Object.defineProperty(${fragment},${_}Symbol.toStringTag,${_}${getToStringTagValue(
getObject
)})`
: fragment;

export const HELPER_NAMES = Object.keys(HELPER_GENERATORS);

export function getToStringTagValue(getObject: GenerateCodeSnippets['getObject']) {
return getObject([['value', "'Module'"]], {
lineBreakIndent: null
});
}

0 comments on commit b74cb92

Please sign in to comment.