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.

Sign up for GitHub

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

fix: Fix missing support for named re-exports #53

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Fix missing support for named re-exports #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions vite-plugin-cjs-interop/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,22 @@ import { __cjs_dyn_import__ } from "virtual:cjs-dyn-import";
});
`;

test("transforms re-export", async () => {
const REEXPORT_INPUT = `export { namedX, named2 as renamedX, default } from "foo";`;

const REEXPORT_OUTPUT = `const { namedX: __cjsInteropSpecifier1__, named2: __cjsInteropSpecifier2__ } = __cjsInterop1__?.default?.__esModule ? __cjsInterop1__.default : __cjsInterop1__;
import __cjsInterop1__ from "foo";
export { __cjsInteropSpecifier1__ as namedX, __cjsInteropSpecifier2__ as renamedX, __cjsInterop1__ as default} };`;

const plugin = cjsInterop({ dependencies: ["foo"] });

const output = await (plugin.transform as any)!(REEXPORT_INPUT, "x.js", {
ssr: true,
});

expect(output.code).toBe(REEXPORT_OUTPUT);
});
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test case for the missing CJS interop wrapping, now supported.


const CSS_INPUT = `:root{--mantine-font-family: Open Sans, sans-serif;`;

test("ignore css assets", async () => {
Expand Down
133 changes: 104 additions & 29 deletions vite-plugin-cjs-interop/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,18 @@ export function cjsInterop(options: CjsInteropOptions): Plugin {
const { client = false, apply = "both" } = options;

let sourcemaps = false;

const matchedDependencies: Record<string, boolean> = {};

const matchesDependencies = (value: string) => {
return dependencies.some((dependency) => minimatch(value, dependency));
if (!(value in matchedDependencies)) {
matchedDependencies[value] = dependencies.some((dependency) =>
minimatch(value, dependency),
);
}
return matchedDependencies[value];
Copy link
Author

@sai-cb sai-cb Dec 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unrelated optimization; Some memoizing here speeds the plugin to a good amount. This makes our builds 1s faster with no apparent downside.

};

return {
name: "cjs-interop",
enforce: "post",
Expand All @@ -61,53 +70,119 @@ export function cjsInterop(options: CjsInteropOptions): Plugin {
});

const toBeFixed: any[] = [];
const dynamicImportsToBeFixed: any[] = [];
const preambles: string[] = [];
let hasDynamicImportsToFix = false;
Copy link
Author

@sai-cb sai-cb Dec 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simplicity, all fixes are included in toBeFixed.

The dynamicImportsToBeFixed is turned to a hasDynamicImportsToFix boolean to replace the one case where we used it as such (if dynamicImportsToBeFixed.length)


const { walk } = await walker;

walk(ast as any, {
enter(node) {
if (node.type === "ImportDeclaration") {
if (matchesDependencies(node.source.value as string)) {
toBeFixed.push(node);
}
} else if (node.type === "ImportExpression") {
if (
node.source.type === "Literal" &&
matchesDependencies(node.source.value as string)
) {
dynamicImportsToBeFixed.push(node);
}
switch (node.type) {
case "ImportDeclaration":
case "ExportNamedDeclaration":
case "ImportExpression":
if (
node.source &&
node.source.type === "Literal" &&
matchesDependencies(node.source.value as string)
) {
toBeFixed.push(node);
if (node.type === "ImportExpression") {
hasDynamicImportsToFix = true;
}
}
break;
default:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds support for ExportNamedDeclaration and applies the same logic to all nodes to be fixed to avoid duplicating code.

}
},
});

if (
toBeFixed.length === 0 &&
dynamicImportsToBeFixed.length === 0
) {
if (toBeFixed.length === 0) {
return;
}
const bottomUpToBeFixed = toBeFixed.reverse();

const ms = sourcemaps ? new MagicString(code) : null;
let counter = 1;
let specifierCounter = 1;
Copy link
Author

@sai-cb sai-cb Dec 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a unique counter for ExportNamedDeclaration specifiers.

let isNamespaceImport = false;

for (const node of dynamicImportsToBeFixed.reverse()) {
const insertion = ".then(__cjs_dyn_import__)";
if (sourcemaps) {
ms!.appendRight(node.end, insertion);
} else {
code =
code.slice(0, node.end) +
insertion +
code.slice(node.end);
for (const node of bottomUpToBeFixed) {
if (node.type === "ImportExpression") {
const insertion = ".then(__cjs_dyn_import__)";
if (sourcemaps) {
ms!.appendRight(node.end, insertion);
} else {
code =
code.slice(0, node.end) +
insertion +
code.slice(node.end);
}
continue;
}
Copy link
Author

@sai-cb sai-cb Dec 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code was just moved. This is a slight behavior change; We no longer process dynamicImportsToBeFixed before bottomUpToBeFixed. Instead, we process all fixes in the (reversed) order that they are found in the module... Which seemed actually safer to me. But please review carefully.


if (node.type === "ExportNamedDeclaration") {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the code inside that if block is the new feature being added to handle re-exports.

const importDestructurings = [];
const exportDestructurings = [];
const name = `__cjsInterop${counter++}__`;
let changed = false;
let defaultExportSpecifier = null;
for (const specifier of node.specifiers) {
if (specifier.type === "ExportSpecifier") {
if (specifier.local.name === "default") {
defaultExportSpecifier = specifier;
continue;
}
changed = true;
const specifierName = `__cjsInteropSpecifier${specifierCounter++}__`;
importDestructurings.push(
`${specifier.local.name}: ${specifierName}`,
);
exportDestructurings.push(
`${specifierName} as ${specifier.exported.name}`,
);
} else {
throw new Error(
`Unknown ExportNamedDeclaration type specifier: ${specifier.type}`,
);
}
}
if (!changed) {
continue;
}
if (defaultExportSpecifier) {
exportDestructurings.push(
`${name} as ${defaultExportSpecifier.exported.name}}`,
);
}
preambles.push(
`const { ${importDestructurings.join(
", ",
)} } = ${name}?.default?.__esModule ? ${name}.default : ${name};`,
);
const replacementNamedImports = `import ${name} from ${JSON.stringify(
node.source.value,
)};`;
const replacementNamedExports = `export { ${exportDestructurings.join(", ")} };`;

const replacement = [
replacementNamedImports,
replacementNamedExports,
]
.filter(Boolean)
.join("\n");

if (ms) {
ms.overwrite(node.start, node.end, replacement);
} else {
code =
code.slice(0, node.start) +
replacement +
code.slice(node.end);
}
continue;
}
}

for (const node of bottomUpToBeFixed) {
const destructurings: string[] = [];
const name = `__cjsInterop${counter++}__`;
let changed = false;
Expand Down Expand Up @@ -162,7 +237,7 @@ export function cjsInterop(options: CjsInteropOptions): Plugin {
}
}

if (dynamicImportsToBeFixed.length) {
if (hasDynamicImportsToFix) {
const importCompat = `import { __cjs_dyn_import__ } from "${virtualModuleId}";\n`;
if (sourcemaps) {
ms!.prepend(importCompat);
Expand Down