Draft
Conversation
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>
|
Cursor Agent can help with this pull request. Just |
Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>
- Add 'use node' directive to import_tests.ts since dynamic imports work in Node.js - Remove dynamicImportQuery which tested runtime failure (now caught at build time) - Remove test_query_dynamic_import Rust test (no longer applicable) The build-time validation replaces runtime errors with clearer deploy-time errors, improving DX. Tests for runtime dynamic import failures in V8 isolates are no longer needed since the bundler now prevents these cases. Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>
Dynamic imports work fine in V8 isolate actions but fail in queries/mutations. Updated the validation to only block dynamic imports in files that import query, mutation, internalQuery, or internalMutation. This preserves existing behavior where dynamic imports work in isolate actions while still catching the case where they would fail at runtime in queries. - Added importsQueryOrMutation() helper to detect query/mutation imports - Updated hasDynamicImport check to also require query/mutation imports - Restored original import_tests.ts without 'use node' since actions work - Restored test_query_dynamic_import Rust test Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>
The dynamicImportQuery test verified runtime failure of dynamic imports in queries. Since files containing query/mutation imports with dynamic imports are now blocked at build time, this runtime test is no longer needed. The file now only contains actions (no query import), so it passes validation since dynamic imports work fine in V8 isolate actions. Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>
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
2 participants
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.
Add dynamic import validation to the bundler to reject dynamic imports in isolate files.
The added check introduces a negligible ~2-4% overhead to the total bundling pipeline, primarily due to an effective fast-path optimization that skips AST parsing for most files.
Slack Thread