Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: built file references undeclared import_foo$n#4745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
IWANABETHATGUY merged 1 commit intomainfrom05-30-fix_4740
May 30, 2025

Conversation

IWANABETHATGUY
Copy link
Member

@IWANABETHATGUYIWANABETHATGUY commentedMay 30, 2025•
edited
Loading

@IWANABETHATGUYGraphite App
Copy link
MemberAuthor

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

@netlifyNetlify
Copy link

netlifybot commentedMay 30, 2025•
edited
Loading

✅ Deploy Preview forrolldown-rs canceled.

NameLink
🔨 Latest commitd94540c
🔍 Latest deploy loghttps://app.netlify.com/projects/rolldown-rs/deploys/68398afa6ad02b0008a434ef

@IWANABETHATGUYIWANABETHATGUY marked this pull request as ready for reviewMay 30, 2025 10:26
@CopilotCopilotAI review requested due to automatic review settingsMay 30, 2025 10:26
Copy link
Contributor

@CopilotCopilotAI 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 ("fix: 4740") introduces updates to test snapshots and several test files for CommonJS compatibility along with minor changes in the code splitting and scope hoisting stages. Key changes include:

  • Updating snapshot metadata for file naming with hash.
  • Adding new test files under the partial_cjs_ns_merge_2 directory.
  • Inserting debug statements in the code splitting stage and refining conditions in scope hoisting.

Reviewed Changes

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

FileDescription
crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snapUpdated snapshot to reflect new test output for module renaming.
crates/rolldown/tests/rolldown/cjs_compat/partial_cjs_ns_merge_2/*New test files added to exercise partial CommonJS namespace merge behavior.
crates/rolldown/src/stages/generate_stage/code_splitting.rsAdded debugging statements which may have been left in unintentionally.
crates/rolldown/src/module_finalizers/scope_hoisting/mod.rsRefined conditional check for merging CJS namespaces.
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (4)

crates/rolldown/tests/rolldown/cjs_compat/partial_cjs_ns_merge_2/main.js:4

  • The variable 'Typography' is referenced but not defined in the code. Either define 'Typography' or remove it from the log statement.
console.log('r', React, Typography, toArray);

crates/rolldown/tests/rolldown/cjs_compat/partial_cjs_ns_merge_2/lib.js:5

  • The variable 'ret' is used without a prior definition. Define and populate 'ret' or return a correctly defined value.
return ret;

crates/rolldown/src/stages/generate_stage/code_splitting.rs:192

  • [nitpick] Debug statements like dbg! should be removed before committing production code.
dbg!(&self.link_output.module_table[*k].id());

crates/rolldown/src/stages/generate_stage/code_splitting.rs:204

  • [nitpick] Consider removing this debug statement to avoid leaving development code in production.
dbg!(&self.link_output.module_table[owner].id());

@IWANABETHATGUYIWANABETHATGUY changed the titlefix: 4740fix: built file references undeclared import_foo$nMay 30, 2025
@IWANABETHATGUYIWANABETHATGUYforce-pushed the05-30-fix_4740 branch 2 times, most recently fromcb3f613 to9362bcdCompareMay 30, 2025 10:39
@github-actionsGitHub Actions
Copy link
Contributor

Benchmarks Rust

group                                 pr                                     target-----                                 --                                     ------bundle/bundle@rome_ts                 1.00    113.3±2.36ms        ? ?/sec    1.00    113.8±2.56ms        ? ?/secbundle/bundle@rome_ts-sourcemap       1.00    127.4±1.71ms        ? ?/sec    1.01    128.3±2.09ms        ? ?/secbundle/bundle@threejs                 1.01     41.2±2.19ms        ? ?/sec    1.00     40.7±2.83ms        ? ?/secbundle/bundle@threejs-sourcemap       1.02     48.4±1.33ms        ? ?/sec    1.00     47.5±0.90ms        ? ?/secbundle/bundle@threejs10x              1.01    433.5±9.55ms        ? ?/sec    1.00    429.9±6.21ms        ? ?/secbundle/bundle@threejs10x-sourcemap    1.04   514.2±10.07ms        ? ?/sec    1.00    496.8±4.41ms        ? ?/secscan/scan@rome_ts                     1.02     88.1±1.59ms        ? ?/sec    1.00     86.6±1.49ms        ? ?/secscan/scan@threejs                     1.02     31.0±0.41ms        ? ?/sec    1.00     30.3±1.93ms        ? ?/secscan/scan@threejs10x                  1.00    318.2±5.72ms        ? ?/sec    1.00    317.6±6.15ms        ? ?/sec

@IWANABETHATGUYIWANABETHATGUY added this pull request to themerge queueMay 30, 2025
Merged via the queue intomain with commitcb30e40May 30, 2025
22 checks passed
@IWANABETHATGUYIWANABETHATGUY deleted the 05-30-fix_4740 branchMay 30, 2025 14:14
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

Copilot code reviewCopilotCopilot left review comments

@hyf0hyf0hyf0 approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: built file references undeclaredimport_foo$n
2 participants
@IWANABETHATGUY@hyf0
[8]ページ先頭
©2009-2025 Movatter.jp