Skip to content

Commit

Permalink
fix(commonjs): treat moduleSideEffects as __PURE__ comments for proxi…
Browse files Browse the repository at this point in the history
…ed wrapped modules
  • Loading branch information
lukastaegert committed Jan 19, 2024
1 parent ccdf1c5 commit 3d78403
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 7 deletions.
10 changes: 7 additions & 3 deletions packages/commonjs/src/index.js
@@ -1,4 +1,4 @@
import { extname, relative, resolve, dirname } from 'path';
import { dirname, extname, relative, resolve } from 'path';

import { createFilter } from '@rollup/pluginutils';

Expand Down Expand Up @@ -239,7 +239,7 @@ export default function commonjs(options = {}) {
}
},

load(id) {
async load(id) {
if (id === HELPERS_ID) {
return getHelpersModule();
}
Expand Down Expand Up @@ -285,7 +285,11 @@ export default function commonjs(options = {}) {

if (isWrappedId(id, ES_IMPORT_SUFFIX)) {
const actualId = unwrapId(id, ES_IMPORT_SUFFIX);
return getEsImportProxy(actualId, getDefaultIsModuleExports(actualId));
return getEsImportProxy(
actualId,
getDefaultIsModuleExports(actualId),
(await this.load({ id: actualId })).moduleSideEffects
);
}

if (id === DYNAMIC_MODULES_ID) {
Expand Down
6 changes: 3 additions & 3 deletions packages/commonjs/src/proxies.js
Expand Up @@ -57,21 +57,21 @@ export function getEntryProxy(id, defaultIsModuleExports, getModuleInfo, shebang
}
return shebang + code;
}
const result = getEsImportProxy(id, defaultIsModuleExports);
const result = getEsImportProxy(id, defaultIsModuleExports, true);
return {
...result,
code: shebang + result.code
};
}

export function getEsImportProxy(id, defaultIsModuleExports) {
export function getEsImportProxy(id, defaultIsModuleExports, moduleSideEffects) {
const name = getName(id);
const exportsName = `${name}Exports`;
const requireModule = `require${capitalize(name)}`;
let code =
`import { getDefaultExportFromCjs } from "${HELPERS_ID}";\n` +
`import { __require as ${requireModule} } from ${JSON.stringify(id)};\n` +
`var ${exportsName} = ${requireModule}();\n` +
`var ${exportsName} = ${moduleSideEffects ? '' : '/*@__PURE__*/ '}${requireModule}();\n` +
`export { ${exportsName} as __moduleExports };`;
if (defaultIsModuleExports === true) {
code += `\nexport { ${exportsName} as default };`;
Expand Down
@@ -0,0 +1,24 @@
module.exports = {
description: 'respects module-side-effects when importing wrapped dependencies',
options: {
plugins: [
{
name: 'test',
async resolveId(source, importer, options) {
if (source.endsWith('./foo.js')) {
const resolved = await this.resolve(source, importer, options);
return { ...resolved, moduleSideEffects: false };
}
return null;
}
}
]
},
pluginOptions: {
strictRequires: true
},
global: (global, t) => {
t.is(global.foo, undefined);
t.is(global.bar, 'bar');
}
};
@@ -0,0 +1 @@
global.bar = 'bar';
@@ -0,0 +1 @@
global.foo = 'foo';
@@ -0,0 +1,2 @@
import { foo } from './foo.js';
import { bar } from './bar.js';
26 changes: 25 additions & 1 deletion packages/commonjs/test/snapshots/function.js.md
Expand Up @@ -6545,6 +6545,30 @@ Generated by [AVA](https://avajs.dev).
`,
}

## module-side-effects-import-wrapped

> Snapshot 1
{
'main.js': `'use strict';␊
var commonjsGlobal = typeof globalThis !== 'undefined' ? globalThis : typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {};␊
var bar = {};␊
var hasRequiredBar;␊
function requireBar () {␊
if (hasRequiredBar) return bar;␊
hasRequiredBar = 1;␊
commonjsGlobal.bar = 'bar';␊
return bar;␊
}␊
requireBar();␊
`,
}

## module-side-effects-late-entry

> Snapshot 1
Expand Down Expand Up @@ -6613,7 +6637,7 @@ Generated by [AVA](https://avajs.dev).
`,
}

## module-side-effects-wrapped
## module-side-effects-require-wrapped

> Snapshot 1
Expand Down
Binary file modified packages/commonjs/test/snapshots/function.js.snap
Binary file not shown.

0 comments on commit 3d78403

Please sign in to comment.