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

getDefaultLibFilePath returns with mixed OS separators on Windows (both \\ and /) #49050

Closed
agilgur5 opened this issue May 10, 2022 · 0 comments · Fixed by #49051
Closed

getDefaultLibFilePath returns with mixed OS separators on Windows (both \\ and /) #49050

agilgur5 opened this issue May 10, 2022 · 0 comments · Fixed by #49051

Comments

@agilgur5
Copy link
Contributor

agilgur5 commented May 10, 2022

Bug Report

Quick summary:

Hi there 👋 , by way of introduction, I help maintain rollup-plugin-typescript2, which is a popular TS Rollup plugin and uses the LanguageService API under-the-hood. As such this is more a LanguageService API / Compiler API internal bug, not necessarily a user-facing one.
(I also previously maintained TSDX, contribute to TypeScript-Website a bunch, tsconfig/bases, etc -- but somehow this is my first issue and PR in TS itself, so hello!)

I was finishing up a unit test suite in ezolenko/rollup-plugin-typescript2#321 when I discovered that getDefaultLibFilePath behaves a bit wacky (ezolenko/rollup-plugin-typescript2#321 (comment)) on Windows.
Spent a couple hours debugging before checking the TS codebase (I totally did not expect a bug in the TS codebase) and quickly found the issue here.

🔎 Search Terms

getDefaultLibFilePath and getDefaultLibFileName (due to #35318)

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about getDefaultLibFilePath

This code appears to be at least ~7 years old according to the blame.

⏯ Playground Link

Playground link with relevant code

This doesn't seem to output anything in the Playground, and as it only occurs on Windows, I'm not sure that a browser will pick it up anyway.

Per my summary above, can see ezolenko/rollup-plugin-typescript2#321 (comment) for a reproduction and I already found the bug in the code here on this line

💻 Code

import ts from 'typescript'

console.log(ts.getDefaultLibFilePath({}))

🙁 Actual behavior

Per ezolenko/rollup-plugin-typescript2#321 (comment), I had a unit test for our Language Service Host implementation, and it uses getDefaultLibFilePath to implement getDefaultLibFileName, same as shown in the Wiki docs for the Compiler API.
Instead of returning a path with either Windows or Linux path separators, it ended up returning a path with both:

    Expected: "D:\\a\\rollup-plugin-typescript2\\rollup-plugin-typescript2\\node_modules\\typescript\\lib\\lib.d.ts"
    Received: "D:\\a\\rollup-plugin-typescript2\\rollup-plugin-typescript2\\node_modules\\typescript\\lib/lib.d.ts"

This only occurs on Windows, tests pass on Ubuntu and Mac.
Possibly worth noting, the rest of the test suite passes, this is the only thing that was failing and that I could not fix on my end, as it's actually due to a bug in TS itself, which I totally did not expect (as the amount of time I spent debugging shows).

🙂 Expected behavior

Since TS represents paths with forward slash (/) internally, I imagine this should output:

D:/a/rollup-plugin-typescript2/rollup-plugin-typescript2/node_modules/typescript/lib/lib.d.ts

Instead of back-slashes (\\), or worse, mixed path separators, as occurred here.

Can see my PR #49051 for a quick fix for this

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 a pull request may close this issue.

1 participant