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

Replace source-map with @jridgewell/trace-mapping #185

Closed
SimenB opened this issue Apr 7, 2022 · 5 comments · Fixed by #186
Closed

Replace source-map with @jridgewell/trace-mapping #185

SimenB opened this issue Apr 7, 2022 · 5 comments · Fixed by #186

Comments

@SimenB
Copy link
Member

SimenB commented Apr 7, 2022

According to its benchmarks, it's way quicker, and it doesn't require us to use async code. Main motivation however, is that 0.7 of source-map is completely broken on the upcoming node v18: nodejs/node#42638

I tried a quick migration, but v8-to-istanbul uses consumer.generatedPositionFor, which doesn't exist in @jridgewell/trace-mapping.

/cc @jridgewell

@jridgewell
Copy link
Contributor

I currently only track from generated -> original positions. I think I can add a lazy initialize for original -> generated position without affecting, which should still be fast. Although, you're only usage of generatedPositionFor is just to get the position of the next segment range. I could easily expose that as a method, which would be considerably faster.

@SimenB
Copy link
Member Author

SimenB commented Apr 7, 2022

If it can be done in a better way, that's of course preferable 😀 I doubt @bcoe is particularly bound to the current implementation

@jridgewell
Copy link
Contributor

Turns out there's not a faster algorithm that we can do. I misunderstood the API you were trying to implement, and in order to find the start of the next segment in source order, you need to use generatedPositionFor. It's now implemented. I'll do a release later today or Mon

@SimenB
Copy link
Member Author

SimenB commented Apr 19, 2022

Sweet, thanks!

@SimenB

This comment was marked as outdated.

jridgewell added a commit to jridgewell/v8-to-istanbul that referenced this issue Apr 20, 2022
[trace-mapping](https://github.com/jridgewell/trace-mapping) is faster, smaller, and lighter than the `source-map` package, and doesn't require WASM, manual memory management, or async/await to use.

Fixes istanbuljs#185.
jridgewell added a commit to jridgewell/v8-to-istanbul that referenced this issue Apr 20, 2022
[trace-mapping](https://github.com/jridgewell/trace-mapping) is faster, smaller, and lighter than the `source-map` package, and doesn't require WASM, manual memory management, or async/await to use.

Fixes istanbuljs#185.
jridgewell added a commit to jridgewell/v8-to-istanbul that referenced this issue Apr 20, 2022
[trace-mapping](https://github.com/jridgewell/trace-mapping) is faster, smaller, and lighter than the `source-map` package, and doesn't require WASM, manual memory management, or async/await to use.

Fixes istanbuljs#185.
jridgewell added a commit to jridgewell/v8-to-istanbul that referenced this issue Apr 20, 2022
[trace-mapping](https://github.com/jridgewell/trace-mapping) is faster, smaller, and lighter than the `source-map` package, and doesn't require WASM, manual memory management, or async/await to use.

Fixes istanbuljs#185.
jridgewell added a commit to jridgewell/v8-to-istanbul that referenced this issue Apr 20, 2022
[trace-mapping](https://github.com/jridgewell/trace-mapping) is faster, smaller, and lighter than the `source-map` package, and doesn't require WASM, manual memory management, or async/await to use.

Fixes istanbuljs#185.
@bcoe bcoe closed this as completed in #186 Apr 20, 2022
bcoe pushed a commit that referenced this issue Apr 20, 2022
[trace-mapping](https://github.com/jridgewell/trace-mapping) is faster, smaller, and lighter than the `source-map` package, and doesn't require WASM, manual memory management, or async/await to use.

Fixes #185.
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.

2 participants