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

fix: non-cjs module may be unexpectedly resolved due to wrong conditions #2090

Merged
merged 2 commits into from
Apr 28, 2024
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
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 20 additions & 21 deletions src/client/theme-api/useLiveDemo.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { SHOULD_SKIP_LIVEDEMO_ERROR } from '@/constants';
import { useDemo } from 'dumi';
import throttle from 'lodash.throttle';
import {
Expand Down Expand Up @@ -96,7 +97,10 @@ export const useLiveDemo = (
const entryFileName = Object.keys(asset.dependencies).find(
(k) => asset.dependencies[k].type === 'FILE',
)!;
const require = (v: string) => {
// why not require?
// https://github.com/airbnb/babel-plugin-dynamic-import-node/blob/a68388870de822183218515a1adbb3e8d7fa9486/src/utils.js#L24
// if just use require, local variable will overwrite __webpack__require__ in the template method
const liveRequire = (v: string) => {
if (v in context!) return context![v];
throw new Error(`Cannot find module: ${v}`);
};
Expand All @@ -118,14 +122,14 @@ export const useLiveDemo = (

if (renderOpts?.renderer && renderOpts?.compile) {
try {
const exports: AgnosticComponentType = {};
const module = { exports };
const liveExports: AgnosticComponentType = {};
const liveModule = { exports: liveExports };
evalCommonJS(entryFileCode, {
exports,
module,
require,
exports: liveExports,
module: liveModule,
require: liveRequire,
});
const component = await getAgnosticComponentModule(exports);
const component = await getAgnosticComponentModule(liveExports);
if (renderOpts.preflight) {
await renderOpts.preflight(component);
}
Expand All @@ -148,20 +152,19 @@ export const useLiveDemo = (
// skip current task if another task is running
if (token !== taskToken.current) return;

const exports: { default?: ComponentType } = {};
const module = { exports };

const liveExports: { default?: ComponentType } = {};
const liveModule = { exports: liveExports };
// initial component with fake runtime
evalCommonJS(entryFileCode, {
exports,
module,
require,
exports: liveExports,
module: liveModule,
require: liveRequire,
});

const newDemoNode = createElement(
DemoErrorBoundary,
null,
createElement(exports.default!),
createElement(liveExports.default!),
);
const oError = console.error;

Expand All @@ -174,13 +177,9 @@ export const useLiveDemo = (
try {
(await renderToStaticMarkupDeferred)(newDemoNode);
} catch (err: any) {
const shouldSkipError =
err.message.includes(
'Unable to find node on an unmounted component',
) ||
err.message.includes(
'Portals are not currently supported by the server renderer',
);
const shouldSkipError = SHOULD_SKIP_LIVEDEMO_ERROR.some((e) =>
err.message.includes(e),
);
if (!shouldSkipError) throw err;
}
console.error = oError;
Expand Down
7 changes: 7 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,10 @@ export const DEFAULT_DEMO_PLAIN_TEXT_EXTENSIONS = [
];

export const FS_CACHE_DIR = 'node_modules/.cache/dumi';

export const SHOULD_SKIP_LIVEDEMO_ERROR = [
'Unable to find node on an unmounted component',
'#188',
'Portals are not currently supported by the server renderer',
'#257',
];
2 changes: 1 addition & 1 deletion src/features/compile/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default (api: IApi) => {
const loaderPath = require.resolve('../../loaders/markdown');

// support require mjs packages(eg. element-plus/es)
memo.resolve.merge({
memo.resolve.byDependency.set('commonjs', {
conditionNames: ['require', 'node', 'import'],
});

Expand Down