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

Resolve URIs with special characters correctly #3392

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

mofux
Copy link
Contributor

@mofux mofux commented Nov 3, 2022

Previously, model URIs were synced with typescript without encoding (uri.toString(true)), breaking resolution of URIs that had special characters in them, e.g. file:///node_modules/@some/module or file:///C:/test.js. This fix makes sure that typescript sees the correct, encoded file URIs. I've also verified that features like Peek Definition is still working with this fix.

Fixes #3281
Fixes #3236

History:

675844a tried to introduce a fix for this problem, but only provided half the solution. This new patch still solves #2295 which was the original target for that commit, while also fixing the encoding problems entirely.

@krismuniz
Copy link

Bumping this PR as it will fix a bunch of uncaught errors that show up in console while inspecting the TypeScript playground downstream.

Any project using @typescript/ata with monaco-editor will see this issue.

Here's a TypeScript Playground repro link: https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbzgVwM4FMDKMCGN1wC+cAZlBCHAERTo4DGMVA3ALABQH9EAdqvAG1uyHjAC6cALwoM2POgAUABgCUbTu2Gj1QA

Inspect the page and you will see the following error:

Uncaught (in promise) Error: Could not find source file: 'file:///file%3A///node_modules/react/package.json'.
    at Ee (tsWorker.js:269:8118)
    at Object.en [as getSyntacticDiagnostics] (tsWorker.js:269:14274)
    at Um.getSyntacticDiagnostics (tsWorker.js:36129:2574)
    at S.fmr (typescript.azureedge.net/cdn/4.9.4/monaco/min/vs/base/worker/workerMain.js:22:34476)
    at o._handleMessage (typescript.azureedge.net/cdn/4.9.4/monaco/min/vs/base/worker/workerMain.js:22:18089)
    at Object.handleMessage (typescript.azureedge.net/cdn/4.9.4/monaco/min/vs/base/worker/workerMain.js:22:17714)
    at L._handleRequestMessage (typescript.azureedge.net/cdn/4.9.4/monaco/min/vs/base/worker/workerMain.js:22:14546)
    at L._handleMessage (typescript.azureedge.net/cdn/4.9.4/monaco/min/vs/base/worker/workerMain.js:22:13996)
    at L.handleMessage (typescript.azureedge.net/cdn/4.9.4/monaco/min/vs/base/worker/workerMain.js:22:13884)
    at o.onmessage (typescript.azureedge.net/cdn/4.9.4/monaco/min/vs/base/worker/workerMain.js:22:17806)

Screenshot of DevTools console example of error above that shows up while inspecting Typescript Playground


I cloned TypeScript playground locally, patched monaco-editor (particularly this change) and the error went away.

@hediet hediet added this to the January 2023 milestone Jan 31, 2023
@hediet hediet enabled auto-merge January 31, 2023 11:45
@hediet hediet merged commit 5afd16b into microsoft:main Jan 31, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants