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

[Playground] Twoslash highlights don't work with TS 4.6 #2247

Closed
Andarist opened this issue Jan 27, 2022 · 9 comments
Closed

[Playground] Twoslash highlights don't work with TS 4.6 #2247

Andarist opened this issue Jan 27, 2022 · 9 comments
Labels
Playground Issues that affect the Playground

Comments

@Andarist
Copy link
Contributor

Repro:

repro playground

Expected behavior:

It should work as it works for the current latest version of TS (4.5.x): TS playground

Actual behavior:

It fails with this in the console:

Uncaught Error: Cannot read properties of undefined (reading 'length')

@orta
Copy link
Contributor

orta commented Feb 6, 2022

Noted in the upstream PR in monaco/vscode which broke it, you're welcome to either make a workaround in the playground or help get it resolved upstream

@Andarist
Copy link
Contributor Author

Andarist commented Feb 7, 2022

I could work on implementing a workaround here. I'm not 100% sure how to handle this though. From what I understand, based on the response here, we'd have to conditionally register the different shape of the inlay provider here:

monaco.languages.registerInlayHintsProvider(sandbox.language, createTwoslashInlayProvider(sandbox))

The question from my side would be - how to determine which shape should be registered? As far as I can tell this depends on the version of the Monaco that is being used and this is how the URL for that gets constructed:

const urlForMonaco = useLocalCompiler ? "http://localhost:5615/dev/vs" : `https://typescript.azureedge.net/cdn/${tsVersion}/monaco/${version}/vs`

I'm not sure how the TS version relates to the Monaco version in this path. Can I safely assume that loading TS 4.6+ means that a new version of the inlay hints provider has to be created? From what I see the actual (semver~) version of Monaco is not exposed anywhere (I've checked window.sandbox.monaco in the playground).

@orta
Copy link
Contributor

orta commented Feb 7, 2022

I've added a note that Monaco might want to have a version number so that this can be determined safely at runtime, ^

I think switching make-monaco-builds to stop using nightly Monaco and use an older commit for the while is probably the right call for the moment, a correct answer would probably need a change in ms/vscode which propagates down through monaco-editor-core

@Andarist
Copy link
Contributor Author

Andarist commented Feb 7, 2022

So, in a sense - this ticket will get resolved for the time being once microsoft/TypeScript-Make-Monaco-Builds#8 lands and gets redeployed?

@orta
Copy link
Contributor

orta commented Feb 7, 2022

Yes, all the nightlies when it was broke will stay broke, but new nightlies (and releases) will get the static version of Monaco with newer TS

@orta
Copy link
Contributor

orta commented Feb 15, 2022

I'm settling for a different answer after seeing #2267

I think we should update to use the new inlay format in the playground, and switch to only supporting the feature on 4.6+ when that comes out so we don't have to juggle two separate APIs with no obvious way to know which shape to give back to Monaco

@Andarist
Copy link
Contributor Author

Damn, I love this feature! I guess though that once 4.6 gets released I won't have as many use cases for using this in pre-4.6 playgrounds 🤔 Whatever works I guess.

It would be great though if we could push Monaco a little bit to include the version somewhere, cause I think this might be very useful in the future. The sooner we get access to it the easier it will be to overcome API incompatibilities in the feature without removing features from older playgrounds.

Do you have any good coordination plan for this release? I mean - I could create a PR to adjust how the API is used but that would have to sit unmerged until 4.6 release, right?

@orta
Copy link
Contributor

orta commented Feb 15, 2022

Yeah, I have mentioned adding a version number in the runtime in microsoft/monaco-editor#2948 - maybe we could bake it in via a script inside make-monaco-builds but I couldn't see an obvious place to add it from the outside

Yeah, FWIW this is the first time a Monaco API has changed on the playground. And yeah, here's no 4.6 PR for the site yet, I was thinking of making one sometime this week as the 22nd is spec'd for the release day but you're welcome to take a look also: https://github.com/microsoft/TypeScript-Website/blob/v2/docs/New%20TypeScript%20Version.md is the guide

@orta
Copy link
Contributor

orta commented Mar 8, 2022

Confirming that these are now working on production with 4.6 👍🏻 - issue can be closed, thanks @Andarist

@Andarist Andarist closed this as completed Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Playground Issues that affect the Playground
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants