PHANTOM
🇮🇳 IN
Skip to content

Fully include dynamic imports in a try-catch#6254

Merged
lukastaegert merged 2 commits intomasterfrom
try-catch-dynamic-import
Jan 30, 2026
Merged

Fully include dynamic imports in a try-catch#6254
lukastaegert merged 2 commits intomasterfrom
try-catch-dynamic-import

Conversation

@lukastaegert
Copy link
Member

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

Inclusion works differently within a try-catch, so we need to take extra care to just include the entire import target in those cases.

Copilot AI review requested due to automatic review settings January 30, 2026 05:07
@vercel
Copy link

vercel bot commented Jan 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
rollup Ready Ready Preview, Comment Jan 30, 2026 6:20am

Request Review

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where dynamic imports in try-catch blocks result in empty objects. When try-catch deoptimization is enabled, Rollup passes INCLUDE_PARAMETERS (the string 'variables') instead of false to the try block, but the ImportExpression's include method wasn't handling this case properly.

Changes:

  • Added logic to include all exports from dynamic imports when includeChildrenRecursively is truthy
  • Added a test case to verify dynamic imports in try-catch blocks work correctly

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/ast/nodes/ImportExpression.ts Added check to call includeAllExports() when includeChildrenRecursively is truthy in the include method
test/function/samples/try-catch-dynamic-import/main.js Test entry point with dynamic import in try-catch using .then()
test/function/samples/try-catch-dynamic-import/dep.js Test dependency file exporting a value
test/function/samples/try-catch-dynamic-import/_config.js Test configuration verifying the imported value is accessible

@github-actions
Copy link

github-actions bot commented Jan 30, 2026

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#try-catch-dynamic-import

Notice: Ensure you have installed the latest nightly Rust toolchain. If you haven't installed it yet, please see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust.

or load it into the REPL:
https://rollup-7jb7ulykd-rollup-js.vercel.app/repl/?pr=6254

@github-actions
Copy link

github-actions bot commented Jan 30, 2026

Performance report

  • BUILD: 6846ms, 828 MB
    • initialize: 0ms, 24.6 MB (+7%)
    • generate module graph: 2579ms, 633 MB
      • generate ast: 1360ms, 626 MB
    • sort and bind modules: 397ms, 692 MB
    • mark included statements: 3868ms, 828 MB
      • treeshaking pass 1: 2241ms, 832 MB
      • treeshaking pass 2: 458ms, 852 MB
      • treeshaking pass 3: 396ms, 833 MB
      • treeshaking pass 4: 384ms, 860 MB
      • treeshaking pass 5: 381ms, 828 MB
  • GENERATE: 677ms, 933 MB
    • initialize render: 0ms, 828 MB
    • generate chunks: 39ms, 852 MB
      • optimize chunks: 0ms, 844 MB
    • render chunks: 628ms, 906 MB
    • transform chunks: 17ms, 933 MB
    • generate bundle: 0ms, 933 MB

@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.83%. Comparing base (c931d23) to head (85c1da8).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6254   +/-   ##
=======================================
  Coverage   98.83%   98.83%           
=======================================
  Files         273      273           
  Lines       10700    10702    +2     
  Branches     2854     2855    +1     
=======================================
+ Hits        10575    10577    +2     
  Misses         82       82           
  Partials       43       43           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lukastaegert lukastaegert merged commit 416f476 into master Jan 30, 2026
48 checks passed
@lukastaegert lukastaegert deleted the try-catch-dynamic-import branch January 30, 2026 07:02
@github-actions
Copy link

This PR has been released as part of rollup@4.57.1. You can test it via npm install rollup.

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.

(Conditional) Import in try..catch fails to bundle

2 participants