Skip to content

feat: support output.esModule option#1812

Merged
hyf0 merged 35 commits intorolldown:mainfrom
7086cmd:feat/namespace-block
Aug 3, 2024
Merged

feat: support output.esModule option#1812
hyf0 merged 35 commits intorolldown:mainfrom
7086cmd:feat/namespace-block

Conversation

@7086cmd
Copy link
Copy Markdown
Contributor

@7086cmd 7086cmd commented Jul 31, 2024

This PR introduces support for namespace blocks in IIFE or CJS format outputs, treating them as ES Modules instead of relying solely on output.exports being named. @underfin (I'm not in a hurry to work on this as I will likely spend a lot of time on my competition project next month. :)

Modifications

  1. Removed redundant usage of determine_export_mode.
  2. Added render_namespace_block, inspired by Rollup, but implemented based on behavior observed in the REPL.
  3. Added corresponding tests to validate the changes.

Behaviors

  1. Namespace block is disabled if output.exports isn't named.
  2. Namespace block is disabled if output.format is esm or app.
    • esm should always be an ES module.
    • app would never be a module.
  3. Entry Point with Default Export:
    • When output.esModule is set to 'always' (or true for Rollup compatibility), or 'if-default-prop' (similar to Rollup), it will render Object.defineProperty(exports, '__esModule', { value: true }).
    • Left an entry for generatedCode.symbol for future updates.

Rationale for Not Using boolean | 'if-default-prop'

During the conversion from JavaScript to Rust, the program encountered a panic when the source value was a boolean. Overriding true or false with a string was considered inelegant, so this approach was not used.

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 31, 2024

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit a001056
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/66ae349787a7fa0008775a14

@hyf0 hyf0 added the on-hold label Jul 31, 2024
@underfin underfin self-assigned this Jul 31, 2024
@hyf0
Copy link
Copy Markdown
Member

hyf0 commented Jul 31, 2024

I'm refactoring cjs exports generating logic to ensure the correct semantic of live binding. Related changes are going to be on hold.

@7086cmd
Copy link
Copy Markdown
Contributor Author

7086cmd commented Jul 31, 2024

I'm refactoring cjs exports generating logic to ensure the correct semantic of live binding. Related changes are going to be on hold.

Ok. It seems that this PR doesn't have an impact on the actual export behavior :) And it's no hurry to be merged.

@7086cmd 7086cmd changed the title feat: support namespace_block. feat: support namespace block. Aug 1, 2024
@hyf0 hyf0 removed the on-hold label Aug 2, 2024
@7086cmd 7086cmd changed the title feat: support namespace block. feat: support output.esModule option and the namespace block. Aug 3, 2024
@7086cmd 7086cmd changed the title feat: support output.esModule option and the namespace block. feat: support output.esModule option and the namespace marker. Aug 3, 2024
@hyf0 hyf0 changed the title feat: support output.esModule option and the namespace marker. feat: support output.esModule option Aug 3, 2024
Copy link
Copy Markdown
Member

@hyf0 hyf0 left a comment

Choose a reason for hiding this comment

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

LGTM. @underfin cc

Comment thread crates/rolldown/src/utils/chunk/namespace_marker.rs Outdated
Comment thread crates/rolldown/src/utils/chunk/namespace_marker.rs Outdated
@underfin
Copy link
Copy Markdown
Contributor

underfin commented Aug 3, 2024

cc @hyf0 Check assignees' approval Look like is not expected.

@hyf0 hyf0 enabled auto-merge August 3, 2024 13:48
@hyf0
Copy link
Copy Markdown
Member

hyf0 commented Aug 3, 2024

There aren't event like approved. It have to manually re-run or trigger by the assign operation. I'll see how to solve it.

@hyf0 hyf0 added this pull request to the merge queue Aug 3, 2024
Merged via the queue into rolldown:main with commit 4999aed Aug 3, 2024
@7086cmd 7086cmd deleted the feat/namespace-block branch August 7, 2024 04:55
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