Skip to content

Comments

feat: add validation errors for incompatible inlineDynamicImports options#7539

Merged
shulaoda merged 9 commits intomainfrom
copilot/add-invalid-option-errors
Dec 17, 2025
Merged

feat: add validation errors for incompatible inlineDynamicImports options#7539
shulaoda merged 9 commits intomainfrom
copilot/add-invalid-option-errors

Conversation

Copy link
Contributor

Copilot AI commented Dec 17, 2025

Summary: Add INVALID_OPTION errors for inlineDynamicImports conflicts

Successfully implemented validation errors for all requested inlineDynamicImports conflict scenarios:

✅ Completed Tasks

  • Add new InvalidOptionType enum variants for the new validation cases
  • Add error messages for each new variant in invalid_option.rs
  • Add validation logic in prepare_build_context.rs for the new cases
  • Create test case: inlineDynamicImports with multiple inputs
  • Create test case: inlineDynamicImports with preserveModules
  • Create test case: inlineDynamicImports with manualChunks (TypeScript side)
  • Create test case: inlineDynamicImports with advancedChunks
  • Run full test suite to verify all validations work correctly
  • Request code review and address feedback
  • Fix type error in manual chunks test

Validation Errors Implemented

  1. Multiple Inputs (Rust) - Error when inlineDynamicImports: true with multiple input entries
  2. PreserveModules (Rust) - Error when inlineDynamicImports: true with preserveModules: true
  3. AdvancedChunks (Rust) - Error when inlineDynamicImports: true with advancedChunks configured
  4. ManualChunks (TypeScript) - Error when inlineDynamicImports: true with manualChunks function

All tests pass successfully and linting passes.

Original prompt

Add INVALID_OPTION errors for the following cases if it does not exist:

  • output.inlineDynamicImports: true is set when there're multiple inputs (the message should be "multiple inputs are not supported when "output.inlineDynamicImports" is true")
  • output.inlineDynamicImports: true is set when output.preserveModules is set
  • output.inlineDynamicImports: true is set when output.manualChunks is set
  • output.inlineDynamicImports: true is set when output.advancedChunks is set

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Dec 17, 2025
@netlify
Copy link

netlify bot commented Dec 17, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit f5ae25b
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69424adaa115f400086c5a79

Copilot AI changed the title [WIP] Add INVALID_OPTION errors for inlineDynamicImports cases Add validation errors for incompatible inlineDynamicImports options Dec 17, 2025
Copilot AI requested a review from sapphi-red December 17, 2025 04:27
@sapphi-red sapphi-red changed the title Add validation errors for incompatible inlineDynamicImports options feat: add validation errors for incompatible inlineDynamicImports options Dec 17, 2025
@sapphi-red sapphi-red changed the title feat: add validation errors for incompatible inlineDynamicImports options feat: add validation errors for incompatible inlineDynamicImports options Dec 17, 2025
@sapphi-red
Copy link
Member

@copilot fix type error

@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2025

Benchmarks Rust

  • target: main(e966919)
  • pr: copilot/add-invalid-option-errors(f5ae25b)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     73.4±3.04ms        ? ?/sec    1.01     74.3±2.55ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     81.0±2.64ms        ? ?/sec    1.00     81.2±2.47ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    114.6±3.08ms        ? ?/sec    1.03    118.3±4.30ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.01    129.8±2.79ms        ? ?/sec    1.00    128.0±2.46ms        ? ?/sec
bundle/bundle@threejs                                        1.00     44.0±2.58ms        ? ?/sec    1.00     43.9±1.26ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     46.8±1.07ms        ? ?/sec    1.00     46.8±0.81ms        ? ?/sec
bundle/bundle@threejs10x                                     1.02    431.1±7.12ms        ? ?/sec    1.00    423.1±7.72ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.02    491.9±6.45ms        ? ?/sec    1.00    482.5±5.26ms        ? ?/sec
scan/scan@rome_ts                                            1.00     88.4±2.30ms        ? ?/sec    1.02     90.6±1.96ms        ? ?/sec
scan/scan@threejs                                            1.00     30.0±1.19ms        ? ?/sec    1.03     30.8±1.97ms        ? ?/sec
scan/scan@threejs10x                                         1.00    306.6±4.53ms        ? ?/sec    1.01    308.7±4.84ms        ? ?/sec

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot fix type error

Fixed in commit 9f2afaa by adding type annotation error: any to the catchError parameter.

Copilot AI requested a review from sapphi-red December 17, 2025 05:59
@sapphi-red sapphi-red marked this pull request as ready for review December 17, 2025 06:16
@sapphi-red sapphi-red requested review from Copilot and hyf0 and removed request for Copilot and sapphi-red December 17, 2025 06:16
@shulaoda shulaoda merged commit 53058fb into main Dec 17, 2025
27 checks passed
@shulaoda shulaoda deleted the copilot/add-invalid-option-errors branch December 17, 2025 09:18
shulaoda added a commit that referenced this pull request Dec 17, 2025
## [1.0.0-beta.55] - 2025-12-17

### 🚀 Features

- add validation errors for incompatible `inlineDynamicImports` options (#7539) by @Copilot
- rolldown_plugin_vite_reporter: truncate long importer lists in ineffective dynamic import warnings (#7528) by @Copilot
- export `memfs` from `rolldown/experimental` for browser builds (#7490) by @Copilot
- implement `postBanner` and `postFooter` (#7487) by @sevenc-nanashi
- port getLogFilter helper from Rollup for advanced log filtering (#6890) by @taearls
- dev: initialize `rolldown_plugin_lazy_compilation` (#7488) by @hyf0
- enable `experimental.transform_hires_sourcemap: 'boundary'` by default (#7478) by @sapphi-red
- apply merge CJS namespace optimizations more generally (#7475) by @sapphi-red

### 🐛 Bug Fixes

- avoid panic on invalid values for `output.exports`, `output.format`, `output.hash_characters` (#7542) by @sapphi-red
- handle `__proto__` export for module namespace correctly (#7534) by @sapphi-red
- handle `__proto__` export correctly (#7533) by @sapphi-red
- rolldown_plugin_vite_resolve: acquire resolver lock before package json resolution (#7524) by @sapphi-red
- disable syntax transform optimization for `minify: 'dce-only'` (#7520) by @sapphi-red
- arrow function expression keep names (#7519) by @IWANABETHATGUY
- skip symbols that are imported from other module when deconflicting module symbols (#7510) by @IWANABETHATGUY
- preserve class names in assignment expressions with keepNames option (#7491) by @IWANABETHATGUY
- accessor with decorators should be kept (#7499) by @Copilot
- rolldown_plugin_vite_reporter: support `Infinity` for chunk_limit (#7497) by @shulaoda
- change filename template validation errors from UNHANDLEABLE_ERROR to INVALID_OPTION (#7472) by @Copilot
- `no entry found for key` error when merging CJS namespace exports (#7474) by @sapphi-red
- generate output with syntax error when wrapped esm module using tla syntax (#7468) by @IWANABETHATGUY

### 🚜 Refactor

- dev: rename `experimental.hmr` to `experimental.devMode` (#7527) by @hyf0
- pass addon option to rust as it is (#7526) by @hyf0
- remove unused stmtinfo meta flag (#7518) by @IWANABETHATGUY

### 📚 Documentation

- clarify `closeBundle` behavior (#7525) by @sapphi-red

### 🧪 Testing

- add tests for static string postBanner and postFooter (#7516) by @Copilot
- disable pluginTimings in Rust integration tests to avoid snapshot noise (#7485) by @shulaoda
- port Rollup JSX tests to `crates/rolldown/tests/rollup` (#7480) by @sapphi-red
- re-triage some esbuild JSX preserve tests (#7479) by @sapphi-red
- disable pluginTimings by default to avoid snapshot noise (#7471) by @shulaoda

### ⚙️ Miscellaneous Tasks

- mark feature that will not be supported as ignored in rollup test status (#7535) by @sapphi-red
- support `banner`/`footer`/`intro`/`outro` for config variants (#7532) by @sapphi-red
- `dce/dce_of_decorators` esbuild test is now passing (#7531) by @sapphi-red
- deps: update github-actions (major) (#7495) by @renovate[bot]
- deps: update npm packages (#7493) by @renovate[bot]
- add a check to verify failedReasons are updated (#7521) by @sapphi-red
- deps: update test262 submodule for tests (#7523) by @sapphi-red
- deps: update taiki-e/install-action action to v2.63.2 (#7496) by @renovate[bot]
- deps: update oxc apps (#7511) by @renovate[bot]
- deps: update oxc to v0.103.0 (#7513) by @camc314
- fix format in `.github/instructions/bug-investigation.instructions.md` (#7506) by @sapphi-red
- add instructions for REPL decoding (#7502) by @sapphi-red
- deps: update github-actions (#7492) by @renovate[bot]
- deps: cargo-shear v1.9.0 (#7483) by @Boshen
- deps: update dependency oxlint-tsgolint to v0.9.0 (#7484) by @renovate[bot]
- deps: update dependency oxlint-tsgolint to v0.8.6 (#7470) by @renovate[bot]
- add syntax validation for test output chunks that are not executed (#7466) by @IWANABETHATGUY
- deps: update dependency rust to v1.92.0 (#7467) by @renovate[bot]
- deps: update test262 submodule for tests (#7457) by @sapphi-red

### ❤️ New Contributors

* @sevenc-nanashi made their first contribution in [#7487](#7487)
* @taearls made their first contribution in [#6890](#6890)

Co-authored-by: shulaoda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants