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

ref(injection): Rebuild fixup_js_file on top of magic-string and sourcemap composition #1693

Merged
merged 16 commits into from Aug 3, 2023

Conversation

loewenheim
Copy link
Contributor

@loewenheim loewenheim commented Jul 21, 2023

This makes debug id injection into JS files precise.

  1. Find the point where we need to inject by means of a regex that skips a hashbang, comments, empty lines, and a "use strict"; statement.
  2. Use magic-string to insert the debug id code snippet at the injection point and append the //# debugId comment.
  3. Have magic-string give us a sourcemap that maps between the injected and original file. This is really only relevant for the code snippet, the comment is just appended and shouldn't affect mappings anyway.

The sourcemap obtained in (3) is then used to rewrite the original sourcemap using SourceMap::adjust_mappings.

This also minorly refactors fixup_js_file_end.

@loewenheim loewenheim self-assigned this Jul 21, 2023
@loewenheim loewenheim marked this pull request as ready for review August 2, 2023 13:48
@loewenheim loewenheim changed the title ref(injection): Rebuild fixup_js_file on top of magic-string ref(injection): Rebuild fixup_js_file on top of magic-string and sourcemap composition Aug 2, 2023
something else"#;

let m = PRE_INJECT_RE.find(source).unwrap();
dbg!(&source[m.range()]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we actually assert on something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to remove that test when I was writing this. I don't think it serves much of a purpose.

src/utils/sourcemaps.rs Outdated Show resolved Hide resolved
src/utils/sourcemaps.rs Outdated Show resolved Hide resolved
loewenheim and others added 2 commits August 2, 2023 16:51
Co-authored-by: Kamil Ogórek <kamil.ogorek@gmail.com>
Co-authored-by: Kamil Ogórek <kamil.ogorek@gmail.com>
Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

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

That's a beefy change, fingers crossed🤞

@loewenheim loewenheim merged commit e09295f into master Aug 3, 2023
16 checks passed
@loewenheim loewenheim deleted the ref/rework-inject branch August 3, 2023 11:36
loewenheim added a commit that referenced this pull request Aug 3, 2023
loewenheim added a commit that referenced this pull request Aug 3, 2023
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.

None yet

2 participants