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

fix!: Fix source map #113

Merged
merged 4 commits into from
Jun 18, 2023
Merged

fix!: Fix source map #113

merged 4 commits into from
Jun 18, 2023

Conversation

kota65535
Copy link
Contributor

fix: #101

Currently, this plugin returns the source map combined with the source maps of all previous plugins.
This means that the source maps are combined twice because they are also combined outside the plugins later (here), which leads to incorrect line numbers.

This PR changes to call instrumenter.instrumentSync twice to create the source map and the instrumented code separately.

src/source-map.ts Outdated Show resolved Hide resolved
@iFaxity
Copy link
Owner

iFaxity commented Jun 13, 2023

Hello @kota65535, thanks for the PR.

Nice solution, there is just one thing that might need a bit looking into. The dependency of esprima is a bit dangerous as it's last update was 5 years ago (jquery/esprima#2122). However as there is a package that is based on esprima called espree that seems to have somewhat of the same API.

I will setup some test projects when i have some free time during next week or so to test the solution with both esprima and espree to see if they both solve the issue with the incorrect source mappings.

@kota65535
Copy link
Contributor Author

Hi @iFaxity, thank you for taking a look!
Certainly, espree might be preferable. I've tested it a little and seems to work as well.

@iFaxity
Copy link
Owner

iFaxity commented Jun 18, 2023

Changing this from a fix to a breaking change when merging. As it is hard to know this will impact projects.

@iFaxity iFaxity merged commit d051190 into iFaxity:next Jun 18, 2023
@github-actions
Copy link
Contributor

🎉 This PR is included in version 5.0.0-rc.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@kota65535 kota65535 changed the title fix: Fix source map fix!: Fix source map Jun 19, 2023
iFaxity pushed a commit that referenced this pull request Jul 22, 2023
* fix sourcemap line number

* fix comment

* use espree

* fix function name
iFaxity pushed a commit that referenced this pull request Jul 22, 2023
* fix sourcemap line number

* fix comment

* use espree

* fix function name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sourcemaps incorrect line number
2 participants