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

realpath of entry-point script is not resolved #1004

Closed
Labels

Comments

@cspotcode
Copy link
Collaborator

cspotcode commented Apr 8, 2020

This issue is a messy brain-dump. I'll clean it up later and ping people for review so they can read the clean version instead of this mess :D

Investigate symlink resolution of the entry-point script and how it affects compilation and config file resolution. E.g. something like ts-node-script ./node_modules/.bin/this-is-a-symlink

This is important if a script uses ts-node-script in the shebang and is symlinked to the path via something like npm's installation mechanism. In that case, the symlink is in a totally different directory than the target script. If we use the realpath we will discover the correct tsconfig file. Otherwise we will search for tsconfig relative to the symlink's location.

Matching node's behavior

We can use node's package.json resolution as a model to follow. If it is using realpath then we should probably do the same. It has --preserve-symlinks and --preserve-symlinks-main flags.

The --preserve-symlinks flag does not apply to the main module, which allows node --preserve-symlinks node_module/.bin/ to work. To apply the same behavior for the main module, also use --preserve-symlinks-main. (source)

It sounds like node almost always realpaths the entry-point, so we can do the same, even before we've loaded the config file. From there, we can obey tsconfig's preserveSymlinks.

Questions

  • Do we override tsconfig's preserveSymlinks with node's --preserve-symlinks? Is node's flag query-able at runtime?
  • do we use fs.realpath() or require.resolve to locate the entry-point? Latter is guaranteed to match node's behavior but requires us to register .ts and .tsx extensions before locating entry-point and config file. (complexity)

TODO

  • merge other PR that adds realpath
  • Test coverage for every situation where symlink resolving affects config resolution
    • tests for all other realpath-related tickets to make sure we don't break use-cases
  • Add realpath or require.resolve to entry-point codepath before config file resolution.

The template below is incomplete; I'll fill in with reproductions when I'm able to come back to this.

Related discussion in #995

Expected Behavior

If entry-point script is ./foo/index.ts, which symlinks to ./bar/index.ts, the latter path should be used for config file resolution, similar to how nodejs will locate the script's package.json relative to the realpath instead of the symlink path.

Actual Behavior

TODO

Steps to reproduce the problem

TODO

Minimal reproduction

TODO

Specifications

  • ts-node version:
  • node version:
  • TypeScript version:
  • tsconfig.json, if you're using one:
{}
  • Operating system and version:
  • If Windows, are you using WSL or WSL2?:
@cspotcode cspotcode added the bug label Apr 10, 2020
cspotcode added a commit that referenced this issue Apr 12, 2020
cspotcode added a commit that referenced this issue Apr 12, 2020

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
cspotcode added a commit that referenced this issue Apr 12, 2020
cspotcode added a commit that referenced this issue Apr 12, 2020
cspotcode added a commit that referenced this issue Apr 12, 2020
cspotcode added a commit that referenced this issue Apr 12, 2020
cspotcode added a commit that referenced this issue Apr 12, 2020
cspotcode added a commit that referenced this issue Apr 12, 2020
cspotcode added a commit that referenced this issue Apr 20, 2020
* add failing test for #1004

* Fix #1004

* Tweak scriptMode entrypoint resolver to include all extensions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment