fix(bundle): deduplicate symlinked dependencies in luaBundle output#1684
Open
ChouUn wants to merge 2 commits intoTypeScriptToLua:masterfrom
Open
fix(bundle): deduplicate symlinked dependencies in luaBundle output#1684ChouUn wants to merge 2 commits intoTypeScriptToLua:masterfrom
ChouUn wants to merge 2 commits intoTypeScriptToLua:masterfrom
Conversation
When using pnpm/yarn workspaces, the same physical package can be referenced through different symlink paths (e.g. node_modules/pkg-a and node_modules/pkg-b/node_modules/pkg-a both pointing to the same directory). TSTL's existing deduplication used string-based path comparison, causing the same module to be bundled multiple times under different module keys. Add canonicalizeDependencyPath() which uses fs.realpathSync() to detect when different symlink paths resolve to the same physical file, ensuring each file appears only once in the bundle output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where luaBundle creates duplicate modules when using symlinked dependencies in pnpm/yarn workspace monorepos. The solution adds symlink-aware deduplication by tracking the real filesystem paths of dependencies and ensuring each physical file appears only once in the bundle.
Changes:
- Adds
canonicalizeDependencyPath()method inResolutionContextto resolve symlinks to their real paths and map them to the first-seen path - Adds test fixture and test case to verify symlinked dependencies are deduplicated
- Integrates canonicalization into the existing dependency resolution flow
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/transpilation/resolve.ts | Adds realPathMap field and canonicalizeDependencyPath() method to detect and deduplicate symlinked dependencies by resolving them to canonical paths |
| test/transpile/module-resolution.spec.ts | Adds test suite with beforeAll/afterAll hooks to dynamically create symlinks and verify no duplication occurs in bundle output |
| test/transpile/module-resolution/project-with-symlinked-dependency/main.ts | Test entry point that imports both directly and transitively from shared-lib via consumer module |
| test/transpile/module-resolution/project-with-symlinked-dependency/tsconfig.json | TypeScript configuration for test fixture |
| test/transpile/module-resolution/project-with-symlinked-dependency/shared-lib/index.lua | Shared library implementation that should appear only once in bundle |
| test/transpile/module-resolution/project-with-symlinked-dependency/shared-lib/index.d.ts | TypeScript declarations for shared library |
test/transpile/module-resolution/project-with-symlinked-dependency/main.ts
Show resolved
Hide resolved
- Add fs.mkdirSync safety check before symlink creation - Add fs.existsSync guard before lstatSync in afterAll cleanup - Use literal hyphen in regex instead of unescaped dot Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1683
Problem
When using
luaBundlein a pnpm/yarn workspace monorepo, the same package gets bundled multiple times under different module keys because symlinked dependencies resolve to different filesystem paths.For example, in a workspace where
pkg-cdepends on bothpkg-aandpkg-b, andpkg-balso depends onpkg-a, the bundle output containspkg-atwice:This happens because pnpm creates symlinks like:
The existing deduplication in
resolvedFilesandprocessedDependenciesuses string-based path comparison, so different symlink paths for the same physical file bypass deduplication.Solution
Add
canonicalizeDependencyPath()inResolutionContextwhich usesfs.realpathSync()to detect when different symlink paths point to the same physical file. The first-seen path becomes the canonical path, ensuring each file appears only once in the bundle.This preserves the existing
symlinks: falsesetting in enhanced-resolve (needed fornode_modulespath detection) while adding symlink-aware deduplication at the dependency processing level.Changes
src/transpilation/resolve.ts: AddrealPathMapandcanonicalizeDependencyPath()toResolutionContextproject-with-symlinked-dependency/(symlink created dynamically in test for cross-platform compatibility)All 44 existing module-resolution tests pass with zero regressions.