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

Complete support for multi-source mapping in plugin mode #197

Merged
merged 4 commits into from
Jun 6, 2024

Conversation

piotrtomiak
Copy link
Contributor

Also, support disablement of code assist in associated non-TS files

@piotrtomiak piotrtomiak changed the title Support for multi-source mapping in plugin mode Complete support for multi-source mapping in plugin mode Jun 5, 2024
mapCache.set(
sourceScript.snapshot,
new SourceMap(virtualCode.mappings)
new SourceMap(allMappings, sourceScript.id)
Copy link
Contributor Author

@piotrtomiak piotrtomiak Jun 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using the map for generated file, we need to be able to map from generated range to all of the local and associated mappings.


private sourceCodeOffsetsMemo: MappingMemo | undefined;
private generatedCodeOffsetsMemo: MappingMemo | undefined;

constructor(public readonly mappings: Mapping<Data>[]) { }
constructor(public readonly mappings: ({ source?: T } & Mapping<Data>)[], private scriptId?: T) { }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need SourceMap of the generated file to be able to map from generated offset to any of the local and associated offsets

if (serviceScript) {
for (const [id, _snapshot, map] of language.maps.forEach(serviceScript.code)) {
if (id === sourceId) {
return [serviceScript, sourceScript, map] as const
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We return here target source script to be able to perform analysis, but we keep map from the original source file to properly map from source to generated offset.

@johnsoncodehk
Copy link
Member

@piotrtomiak Thanks for the PR! I will review it soon, can you share any sample repo that can test this behavior?

@piotrtomiak
Copy link
Contributor Author

@johnsoncodehk - it is difficult to share the whole repo, as templates are transpiled on JVM side for performance reasons - you need to build the whole Angular application model first. Here is the plugin project: https://drive.google.com/file/d/1y7hDmtP7l1VsWWR48jxv0PngFGLPGU3X/view?usp=sharing. It is only the TypeScript side, which runs as TSC plugin. You can see that transpiled template is sent to the server with webStormNgUpdateTranspiledTemplate command. I guess you could mock it up, or I can prepare for you a nightly build of WebStorm, where you can check the whole code flow. You can debug the TS server when it's running within WebStorm.

@johnsoncodehk johnsoncodehk merged commit c34e093 into volarjs:master Jun 6, 2024
3 checks passed
@johnsoncodehk
Copy link
Member

johnsoncodehk commented Jun 6, 2024

Here is the plugin project: https://drive.google.com/file/d/1y7hDmtP7l1VsWWR48jxv0PngFGLPGU3X/view?usp=sharing.

Thanks! Very useful. 👍

I've released 2.3.0-alpha.8 for this, please let me know if you find problems, or please feel free to send a PR.

Update: I seem to have broken the use case of doing a find definition jump from .html to .ts and am trying to fix it.

@johnsoncodehk
Copy link
Member

johnsoncodehk commented Jun 6, 2024

Update: I seem to have broken the use case of doing a find definition jump from .html to .ts and am trying to fix it.

It should be fixed by 511b4c1.

johnsoncodehk added a commit that referenced this pull request Jun 7, 2024
@piotrtomiak
Copy link
Contributor Author

@johnsoncodehk - thanks for working on this PR and all the fixes! Really appreciated!

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 this pull request may close these issues.

2 participants