Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer .js over .ts causing over-eager rebuilds with watch mode with preserve-symlinks #3579

Open
znewsham opened this issue Jan 4, 2024 · 4 comments

Comments

@znewsham
Copy link

znewsham commented Jan 4, 2024

I'm having some problems with the 0.18.0 change to prefer js over ts files and how it interacts with the preserveSymlinks option? I'm experiencing these problems on the latest 0.19 version too.

From #3019:

Regarding monorepos with symlinks: Like node, esbuild defaults to using the real path of an input file as its identity. So if there are symlinks in node_modules that point outside of node_modules, the real path of any of those input files won't include node_modules. You can change that behavior with esbuild's --preserve-symlinks setting. So I guess in that case esbuild's behavior would differ depending on the value of that setting, but by default esbuild would prefer .ts files over .js files in that case

This seems to be saying that when using symlinks, if --preserve-symlinks is false, there will be no concerns with the new ts vs js preference, however when true it is expected to cause problems?

My problem is that we have locally symlinked node modules that are shared between applications - in a structure a little like this:

folders: 
shared/<packageName>
services/<appName>/shared -> ../../shared

services/<appName>/package.json
{
 ...
 "dependencies": {
  "mySharedPackageName": "file:shared/mySharedPackageName"
 }
}

Setting preserveSymlinks: false causes the build to fail due to missing packages, not just due to missing local packages, but any NPM dependency of a shared package (regardless of whether it's also local and shared or from NPM) causes esbuild to throw a "Could not resolve" error.

Setting preserveSymlinks: true causes the build to ignore any typescript changes after the initial build (e.g., once esbuild has generated the typescript files).

Is there currently any way to workaround this? E.g., tsconfig, or an option for esbuild itself? Would a change in project structure resolve this problem (while still allowing shared local packages)

Would you support detecting that the file exists outside of node_modules regardless of the preserveSymlinks setting, or a white/blacklist glob pattern option to prefer ts over js in certain cases?

@evanw
Copy link
Owner

evanw commented Jan 4, 2024

I think I'm going to need a reproduction case for this one, sorry. I don't use symlinks heavily myself and can't quite understand the problem you're describing. Please provide instructions and/or sample code to reproduce the problem and describe exactly what you want esbuild to do instead. You may find esbuild's issue reporting instructions helpful:

When reporting a bug or requesting a feature, please do the following:

  • Provide a way to reproduce the issue. The best way to do this is to demonstrate the issue on the playground (https://esbuild.github.io/try/) and paste the URL here. A link to a minimal code sample with instructions for how to reproduce the issue may also work. Issues without a way to reproduce them may be closed.

@znewsham
Copy link
Author

znewsham commented Jan 4, 2024

The repro is fortunately quite simple - if you don't mind an artificial repro, e.g., let's ignore the question of "why do you have JS and TS files together"

znewsham/test-esbuild#1 - if you look in issue-3579 folder the repro should be pretty easy

@evanw
Copy link
Owner

evanw commented Jan 4, 2024

# This creates a build, but uses the JS file of my-package instead of the TS file
npx esbuild --preserve-symlinks --platform=node --bundle --outfile=build/index.js --format=esm index.js

# see "THIS SHOULD NEVER LOG"
node build/index.js

Sorry, I don't understand. This happens both with esbuild 0.17.0 (i.e. before the version 0.18.0 that you mentioned) and with esbuild 0.19.11. That's because package.json contains "main": "index.js" which unambiguously points to the .js file. The logic for falling back to .ts if the .js file is not found never kicks in in the first place here, either in the latest version or in versions before 0.18.0. So this is not the result of any behavior introduced by version 0.18.0.

# This throws an error about missing dependency of my-package
npx esbuild --platform=node --bundle --outfile=build/index.js --format=esm index.js

Isn't that because the my-package dependency of my-package-2 is legitimately missing? The bundling operation succeeds if I change it to point to where your code actually lives, like you did for the esbuild-demo package:

-    "my-package": "^0.0.1"
+    "my-package": "file:../my-package"

@znewsham
Copy link
Author

znewsham commented Jan 4, 2024

I over-simplified a little, my bad. If you check that PR again you'll be able to repro (and NOT repro in 0.17)

Regarding the index.js:

  1. even on 0.18+ if you delete index.js - it works, so esbuild is "correctly" (?) detecting that an index.ts exists and is compiling it to JS - but when the JS file also exists it's ignoring the TS file
  2. it's easy enough to "remove" this problem by making the file test.js/ts and having index.js import ./test

Regarding the my-package dependency - it's a peer dependency, so the expectation is the package is installed by the app, which does have it setup correctly - if I change the peer dependency to point to the relative path I get the exact same error - which is what I'd expect.

@evanw evanw removed the unactionable label Jan 4, 2024
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

No branches or pull requests

2 participants