Skip to content

Commit 8f368b3

Browse files
authored
Fix markbind serve -d regression (#2868)
* Fix `markbind serve -d` Markbind hot reload breaks due to the old import syntax used in updating the Markbind Vue Bundle. Change to ESM-compatible method of writing src to a temporary .cjs file that is imported using the file path. ESM only supports similar require functionality when using its experimental vm.Module object that can only be accessed when ran with a flag. * Fix typo in gitignore * Use createRequire workaround to bundle vue Implementation currently writes to a temp file, which is placed in the directory of the source code so that it can crawl the require tree. This may not work in production environments, especially if markbind-cli was installed globally as a root user and used as a normal user, as it wouldn't be able to write into the directory that markbind lives in. (additionally, we can't write anywhere else as it then wouldn't be able to crawl the dep tree) Use createRequire with an eval workaround to import it instead. This prevents the need of writing to a temp directory, similar to before. However, ESM still does not expose the `module` object, so the same method as before wouldn't work. Therefore, use Function() to import it, passing in the constructed require object. * Remove outdated ignore rule * Add test cases for requireFromString requireFromString is untested and private Due to changes in build output, this method broke silently, highlighting importance of testing this section. Add unit tests checking import functionality. * Move require creation to module scope
1 parent 19b34fe commit 8f368b3

File tree

2 files changed

+91
-18
lines changed

2 files changed

+91
-18
lines changed

packages/core/src/Page/PageVueServerRenderer.ts

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,20 @@
1-
/* eslint-disable import/no-import-module-exports */
2-
/*
3-
Note: the function `requireFromString` causes eslint to detect a false-positive
4-
due to the usage of `module`. Hence, the use of the above `eslint-disable`.
5-
*/
61
import * as Vue from 'vue';
72
import { createSSRApp } from 'vue';
83
import { renderToString } from 'vue/server-renderer';
94
import { compileTemplate } from 'vue/compiler-sfc';
105
import type { SFCTemplateCompileOptions, CompilerOptions } from 'vue/compiler-sfc';
116

7+
import { createRequire } from 'module';
128
import path from 'path';
139
import fs from 'fs-extra';
1410
import vueCommonAppFactory from '@markbind/core-web/dist/js/vueCommonAppFactory.min.js';
15-
1611
import * as logger from '../utils/logger.js';
1712
import type { PageConfig, PageAssets } from './PageConfig.js';
1813
import type { Page } from './index.js';
1914
import { PluginManager } from '../plugins/PluginManager.js';
2015

16+
const require = createRequire(import.meta.url);
17+
2118
let customElementTagsCache: Set<string> | undefined;
2219

2320
let bundle = { ...vueCommonAppFactory };
@@ -91,17 +88,24 @@ async function compileVuePageCreateAndReturnScript(
9188
return outputContent;
9289
}
9390

94-
/**
95-
* Referenced from stackOverflow:
96-
* https://stackoverflow.com/questions/17581830/load-node-js-module-from-string-in-memory
97-
*
98-
* Credits to Dominic
99-
*/
100-
function requireFromString(src: string, filename: string) {
101-
const m = new (module.constructor as any)();
102-
m.paths = module.paths; // without this, we won't be able to require Vue in the string module
103-
m._compile(src, filename);
104-
return m.exports;
91+
function requireFromString(src: string) {
92+
// Use createRequire since bundle is CJS. This allows require() calls within the bundle
93+
// to be resolved relative to this file.
94+
const mod = { exports: {} as any };
95+
96+
// Use Function (like eval) to load bundle in global scope for usage
97+
// How this works: It passes in require from createRequire, the module and exports
98+
// object into the `src` code as parameters. The `src` code then uses these naturally
99+
// and populates the mod object, while using the require() from createRequire to
100+
// load dependencies (which are CJS `require` calls themselves).
101+
try {
102+
// eslint-disable-next-line @typescript-eslint/no-implied-eval
103+
new Function('require', 'module', 'exports', src)(require, mod, mod.exports);
104+
} catch (e) {
105+
logger.error(e);
106+
}
107+
108+
return mod.exports.default ?? mod.exports;
105109
}
106110

107111
/**
@@ -154,7 +158,7 @@ async function updateMarkBindVueBundle(newBundle: string): Promise<void> {
154158
Bundle is regenerated by webpack and built pages are re-rendered with the latest bundle.`);
155159

156160
// reassign the latest updated MarkBindVue bundle
157-
bundle = requireFromString(newBundle, '');
161+
bundle = requireFromString(newBundle);
158162

159163
Object.values(pageEntries).forEach(async (pageEntry) => {
160164
const { page, renderFn } = pageEntry;
@@ -168,4 +172,5 @@ export const pageVueServerRenderer = {
168172
renderVuePage,
169173
updateMarkBindVueBundle,
170174
savePageRenderFnForHotReload,
175+
requireFromString,
171176
};

packages/core/test/unit/Page/PageVueServerRenderer.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,72 @@ describe('PageVueServerRenderer', () => {
8989
expect(isCustomElement('any-tag')).toBe(false);
9090
});
9191
});
92+
93+
describe('requireFromStringMethod', () => {
94+
test('imports CJS javascript code', () => {
95+
const src = `
96+
require('node:fs');
97+
98+
function helloWorld() {
99+
return 'Hello World!';
100+
}
101+
module.exports = { helloWorld };
102+
`;
103+
104+
const module = pageVueServerRenderer.requireFromString(src);
105+
106+
// Assert that helloWorld method is present in module object
107+
expect('helloWorld' in module).toBe(true);
108+
});
109+
110+
test('imports a mock Vue bundle', () => {
111+
const mockVueBundle = `
112+
const Vue = require('vue');
113+
114+
const MarkBindVue = {
115+
plugin: {
116+
install: function(app) {
117+
app.config.globalProperties.$test = 'test';
118+
}
119+
}
120+
};
121+
122+
const appFactory = function() {
123+
return {
124+
data() {
125+
return { test: 'value' };
126+
}
127+
};
128+
};
129+
130+
module.exports = { MarkBindVue, appFactory };
131+
`;
132+
133+
const module = pageVueServerRenderer.requireFromString(mockVueBundle);
134+
const appFactory = module.appFactory();
135+
136+
// Assert that bundle contains expected properties
137+
expect(module).toHaveProperty('MarkBindVue');
138+
expect(module).toHaveProperty('appFactory');
139+
expect(typeof module.MarkBindVue.plugin.install).toBe('function');
140+
expect(typeof module.appFactory).toBe('function');
141+
expect(appFactory).toBeDefined();
142+
});
143+
144+
test('executes gracefully with invalid code', () => {
145+
const invalidSrc = `
146+
(defun hello-world ()
147+
(format t "hello world"))
148+
149+
(hello-world)
150+
`;
151+
152+
// Act: Invalid src should execute and not throw exceptions
153+
const module = pageVueServerRenderer.requireFromString(invalidSrc);
154+
155+
// Assert that module has no exports
156+
// (logger should inform of error)
157+
expect(module).toEqual({});
158+
});
159+
});
92160
});

0 commit comments

Comments
 (0)