Skip to content

fix(bundler): avoid shadowed __dirname in import.meta patch#5916

Merged
killagu merged 1 commit intonextfrom
agent/egg-dev/0a638d6f
May 3, 2026
Merged

fix(bundler): avoid shadowed __dirname in import.meta patch#5916
killagu merged 1 commit intonextfrom
agent/egg-dev/0a638d6f

Conversation

@killagu
Copy link
Copy Markdown
Contributor

@killagu killagu commented May 3, 2026

Summary

  • Avoid reading __dirname inside the generated Turbopack import.meta replacement.
  • Derive patched import.meta.dirname from the resolved filename instead, so later generated lexical const __dirname declarations cannot trigger TDZ.
  • Add an integration regression covering a patched chunk that declares const __dirname after the import.meta object.

Root Cause

The post-pack import.meta patch emitted typeof __dirname === "string" ? __dirname : ....
In cnpmcore's generated root chunk, Turbopack also emits a later const __dirname = ... in the same module scope. That lexical declaration shadows CommonJS __dirname for the whole scope, so even typeof __dirname throws ReferenceError: Cannot access __dirname before initialization before the later declaration runs.

This is a general shadowed-lexical-binding issue in Egg's import.meta output patch, not cnpmcore-specific application code.

Verification

  • pnpm vitest run test/integration.test.ts from tools/egg-bundler
  • pnpm --filter @eggjs/egg-bundler typecheck
  • pnpm --filter @eggjs/egg-bundler lint
  • pnpm --filter @eggjs/egg-bundler test
  • cnpmcore 971784993927 local reproduction with egg@4.1.2-beta.9 dependencies:
    • bundle via local built egg-bundler plus app-level supports-color alias completed and produced dist-bundle-tdz-fix/worker.js
    • timeout 20s node dist-bundle-tdz-fix/worker.js no longer hit the __dirname TDZ blocker
    • new first runtime blocker is Cannot find native binding for @cnpmjs/packument-linux-x64-gnu / packument.linux-x64-gnu.node

Summary by CodeRabbit

  • Bug Fixes

    • Fixed bundled outputs so import metadata derives the directory from the computed filename, ensuring correct behavior when a module-local dirname is present.
  • Tests

    • Updated and added integration tests to verify bundler behavior when dirname is shadowed and when patching import metadata.

Copilot AI review requested due to automatic review settings May 3, 2026 05:24
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2a1c7ec0-4b1a-4ef5-b610-48deb170e824

📥 Commits

Reviewing files that changed from the base of the PR and between ba21354 and ca921a6.

📒 Files selected for processing (2)
  • tools/egg-bundler/src/lib/Bundler.ts
  • tools/egg-bundler/test/integration.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • tools/egg-bundler/src/lib/Bundler.ts
  • tools/egg-bundler/test/integration.test.ts

📝 Walkthrough

Walkthrough

The bundler now generates import.meta.dirname purely from the computed filename (by locating the last '/' or '' and applying slicing rules including drive-letter/root edge cases), removing any conditional runtime fallback to __dirname; import.meta.filename and url remain filename-based.

Changes

Import.meta dirname calculation

Layer / File(s) Summary
Core Implementation
tools/egg-bundler/src/lib/Bundler.ts
#renderImportMetaObject() stops emitting a typeof __dirname === "string" ? __dirname : ... fallback and instead inlines a function that derives dirname from filename using lastIndexOf for '/' and '\' with conditional slicing (handles drive letters and root-like cases), defaulting to "." when no separator is present.
URL / Filename generation (unchanged)
tools/egg-bundler/src/lib/Bundler.ts
url and filename continue to be emitted based on filename (path separator normalization into a file:/// URL).
Integration Tests
tools/egg-bundler/test/integration.test.ts
Removed an assertion that bundle-manifest.chunks excludes *.js.map entries; added a test that runs a bundled chunk in a vm sandbox with a provided __dirname binding and asserts the patched import.meta.dirname equals the sandbox __dirname across filename inputs (verifies shadowed/local __dirname is not used).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰
I nibble paths beneath the moonlit line,
No runtime shadow steals what’s mine.
From filename I carve the place,
A tidy dirname, snug in space.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly addresses the main change: fixing the bundler to avoid issues with shadowed __dirname in the import.meta patch, which is the core motivation for computing dirname from filename instead of reading __dirname.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/egg-dev/0a638d6f

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.04%. Comparing base (5baa1ed) to head (ca921a6).
⚠️ Report is 1 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #5916   +/-   ##
=======================================
  Coverage   85.03%   85.04%           
=======================================
  Files         667      667           
  Lines       19123    19123           
  Branches     3723     3723           
=======================================
+ Hits        16262    16263    +1     
+ Misses       2468     2467    -1     
  Partials      393      393           

☔ 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.

Copy link
Copy Markdown
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 runtime TDZ failure in egg-bundler’s post-pack Turbopack import.meta patching by removing references to __dirname, which can be shadowed by later lexical const __dirname declarations in the same module scope.

Changes:

  • Update the generated Turbopack import.meta replacement to derive import.meta.dirname purely from the resolved filename (avoiding __dirname access).
  • Add an integration regression test covering a chunk that declares const __dirname after the patched import.meta object.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tools/egg-bundler/src/lib/Bundler.ts Adjusts generated import.meta object patch to avoid __dirname TDZ by deriving dirname from filename.
tools/egg-bundler/test/integration.test.ts Adds a regression test ensuring patched chunks don’t touch a shadowed __dirname binding.

Comment thread tools/egg-bundler/src/lib/Bundler.ts Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies the dirname calculation in Bundler.ts by removing the check for a global __dirname and adds an integration test to ensure import.meta patching works correctly when __dirname is shadowed. A review comment points out that the current regex-based calculation returns an empty string for files at the root directory and suggests a fallback to match Node.js behavior.

Comment thread tools/egg-bundler/src/lib/Bundler.ts Outdated
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 3, 2026

Deploying egg with  Cloudflare Pages  Cloudflare Pages

Latest commit: ca921a6
Status: ✅  Deploy successful!
Preview URL: https://826d3b69.egg-cci.pages.dev
Branch Preview URL: https://agent-egg-dev-0a638d6f.egg-cci.pages.dev

View logs

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 3, 2026

Deploying egg-v3 with  Cloudflare Pages  Cloudflare Pages

Latest commit: ca921a6
Status: ✅  Deploy successful!
Preview URL: https://16e04e7c.egg-v3.pages.dev
Branch Preview URL: https://agent-egg-dev-0a638d6f.egg-v3.pages.dev

View logs

@killagu killagu force-pushed the agent/egg-dev/0a638d6f branch from ba21354 to ca921a6 Compare May 3, 2026 05:51
@killagu killagu merged commit 9ae1df0 into next May 3, 2026
37 of 38 checks passed
@killagu killagu deleted the agent/egg-dev/0a638d6f branch May 3, 2026 06:43
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.

2 participants