fix(plugin-runner): emit ESM bundles for Node plugins - #627
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcfcb3810c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
I have read the CLA Document and I hereby sign the CLA |
github-actions
Bot
added
the
CLA signed
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bcc0370a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
548f126 to
144a693
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 144a69372f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1fd6d1714
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "--bundle", | ||
| "--format=esm", | ||
| // Keep CommonJS plugin sources that use Node's module and file globals working in the ESM bundle. | ||
| '--banner:js=import { createRequire as __lmsCreateRequire } from "module"; import { fileURLToPath as __lmsFileURLToPath } from "url"; import { dirname as __lmsDirname } from "path"; const require = __lmsCreateRequire(import.meta.url); const __filename = __lmsFileURLToPath(import.meta.url); const __dirname = __lmsDirname(__filename);', |
There was a problem hiding this comment.
When an ESM plugin already defines the customary createRequire/fileURLToPath polyfills for require, __filename, or __dirname, this banner is inserted verbatim without participating in esbuild's symbol renaming. The bundled plugin declarations can therefore share the output scope with these const declarations, causing the generated dev.js or production.js to fail parsing with an identifier-already-declared error; inject collision-safe symbols or only provide these bindings when the bundle does not define them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6f27c2f. The ESM banner no longer declares top-level require, __filename, or __dirname; it defines missing bindings on globalThis instead, so plugin-provided ESM polyfills do not cause duplicate declaration errors while CommonJS globals remain available. Added a runtime regression that executes the banner alongside plugin-provided bindings. Focused Jest: 3 passed; ESLint: passed; git diff --check: passed. TypeScript build remains blocked by the existing clean-checkout workspace-link issue for @lmstudio/lms-common and @lmstudio/lms-common-server.
1 participant
Summary
Problem
Node plugins with
"type": "module"failed at runtime because esbuild's default Node format generated CommonJSrequirecalls for.lmstudio/dev.jsand.lmstudio/production.js.Call graph
packages/lms-es-plugin-runner/src/esbuildArgs.ts:16(createEsBuildArgs)→
NodePluginInstaller/NodePluginRunnerWatcher→ esbuild output
.lmstudio/dev.jsor.lmstudio/production.jsThe shared argument builder now passes
--format=esm, so the generated files load correctly in ESM plugin packages.Tests
npm test -- --runInBand --coverage=false --silentnpx turbo run build --filter=@lmstudio/lms-es-plugin-runner...npx tsc --build packages/lms-es-plugin-runner/tsconfig.jsonnpx prettier --check packages/lms-es-plugin-runner/src/esbuildArgs.ts packages/lms-es-plugin-runner/src/esbuildArgs.test.tsnpx eslint packages/lms-es-plugin-runner/src/esbuildArgs.ts packages/lms-es-plugin-runner/src/esbuildArgs.test.tsgit diff --checkFixes #586
Review update validation
npm test -- --runInBand --coverage=false --silent packages/lms-es-plugin-runner/src/esbuildArgs.test.ts— 2 passednpx prettier --check packages/lms-es-plugin-runner/src/esbuildArgs.ts packages/lms-es-plugin-runner/src/esbuildArgs.test.ts— passednpx eslint packages/lms-es-plugin-runner/src/esbuildArgs.ts packages/lms-es-plugin-runner/src/esbuildArgs.test.ts— passedgit diff --check— passednpx tsc --build packages/lms-es-plugin-runner/tsconfig.json— not completed in this clean checkout becausenpm ci's repository postinstall could not find the@lmstudio/lms-cliworkspace, leaving@lmstudio/lms-commonand@lmstudio/lms-common-serverunlinked; the focused test compiles and executes the banner under Node.