Block dynamic imports in v8 isolate Convex files at deploy time#379
Open
ghandic wants to merge 1 commit intoget-convex:mainfrom
Open
Block dynamic imports in v8 isolate Convex files at deploy time#379ghandic wants to merge 1 commit intoget-convex:mainfrom
ghandic wants to merge 1 commit intoget-convex:mainfrom
Conversation
Member
|
Some libraries use this to dynamically pull in the right files - and I
believe esbuild tries to detect these and bundle them in.
What issue were you seeing that led you to suggest this change?
…On Thu, Feb 26, 2026 at 1:53 PM Andy Challis ***@***.***> wrote:
Summary
- Add deploy-time validation that rejects import() expressions in
Convex files that don't have a "use node" directive. Previously this was
only caught at runtime in the V8 isolate, leading to a bad DX.
- The check runs during bundling in entryPointsByEnvironment(), before
esbuild, using @babel/parser AST analysis with a fast string pre-check.
- Test files (.test.ts, .spec.ts) are already excluded from entry
points by existing bundler logic and are unaffected.
Test plan
- Added unit tests for hasDynamicImport() covering: top-level imports,
nested in functions, template literals, TypeScript, static-only imports,
comments, and string literals.
- Added integration tests verifying entryPointsByEnvironment() rejects
isolate files with dynamic imports and allows them in "use node" files.
------------------------------
By submitting this pull request, I confirm that you can use, modify, copy,
and redistribute this contribution, under the terms of your choice.
------------------------------
You can view, comment on, or merge this pull request online at:
#379
Commit Summary
- 5dec8c6
<5dec8c6>
catch dynamic imports before deployment
File Changes
(4 files <https://github.com/get-convex/convex-backend/pull/379/files>)
- *M* npm-packages/convex/src/bundler/index.test.ts
<https://github.com/get-convex/convex-backend/pull/379/files#diff-e46b18684d1cde4b89b1843f3189953d38365d6a96ba77c05ae9d4544b49d067>
(101)
- *M* npm-packages/convex/src/bundler/index.ts
<https://github.com/get-convex/convex-backend/pull/379/files#diff-3374941a06f66c232634e599f5072cb443a429389d4c937c8db8164eb7cd971d>
(52)
- *A*
npm-packages/convex/src/bundler/test_fixtures/js/project_with_dynamic_import/actions.js
<https://github.com/get-convex/convex-backend/pull/379/files#diff-c09fa6914222d0cea6e62d7cdb86e92f8c77eec34b0d4f78a1437bcda8914b1a>
(4)
- *A*
npm-packages/convex/src/bundler/test_fixtures/js/project_with_dynamic_import_use_node/actions.js
<https://github.com/get-convex/convex-backend/pull/379/files#diff-c27a8f400de5ee3df4376bde469516a2a9ec3c5b4a8f33c359ef78b9c990552f>
(6)
Patch Links:
- https://github.com/get-convex/convex-backend/pull/379.patch
- https://github.com/get-convex/convex-backend/pull/379.diff
—
Reply to this email directly, view it on GitHub
<#379>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACZQW4SWEPDI4A3XA5FKL34N5TLXAVCNFSM6AAAAACWBB7N2CVHI2DSMVQWIX3LMV43ASLTON2WKOZTHE4TOOJUGIYDKMI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Author
|
Dynamic imports fail at runtime on convex but don't get caught at build. I've had agents add this in and I normally find it quickly before it matters but people in discord have had similar issues. If it's blocked at runtime why not build time ? |
Member
|
I wasn't aware the dynamic imports always failed - I thought they'd succeed
if the file is bundled. Will this add significant latency if they have 30MB
of functions, say? Any benchmarks on the bundle latency or sense of the
impact?
|
Author
|
Should be low impact as it does a string match first and only runs if there's import( |
cursor bot
pushed a commit
that referenced
this pull request
Mar 3, 2026
This is a copybara of #379. Changes: - Add deploy-time validation that rejects import() expressions in Convex files that don't have a "use node" directive - The check runs during bundling in entryPointsByEnvironment(), using @babel/parser AST analysis with a fast string pre-check - Test files (.test.ts, .spec.ts) are already excluded from entry points by existing bundler logic and are unaffected Performance impact (benchmarked): - Fast-path optimization: files without 'import(' string skip AST parsing - Typical overhead: 2-4% of total bundling time (~1-5ms for 50-200 files) - esbuild dominates total time (20-50ms), so check overhead is negligible Tests: - Added unit tests for hasDynamicImport() covering various patterns - Added integration tests for entryPointsByEnvironment() behavior Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>
Member
|
It both works and doesn't work 😅 this is failing for this file that currently tests that dynamic imports work (perhaps only if they're explicitly imported elsewhere?): import * as helpersStatic from "./helpers";
export const dynamicImport = action({
args: {},
handler: async () => {
const helpers = await import("./helpers");
assert.strictEqual(helpers.fibonacci(6), 8.0);
// The `import * as helpersStatic` repackages the namespace object, so we
// can't assert helpersStatic === helpers, but we can check that the module
// was not re-evaluated by checking equality of a field.
assert.strictEqual(helpersStatic.fibonacci, helpers.fibonacci);
const helpersAgain = await import("./helpers");
assert.strictEqual(helpers, helpersAgain);
const helpersDifferentPath = await import("./helpers");
assert.strictEqual(helpers, helpersDifferentPath);
},
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.