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(inject): Inject code snippet at the start #1567

Merged
merged 16 commits into from
May 24, 2023
Merged

Conversation

loewenheim
Copy link
Contributor

@loewenheim loewenheim commented Apr 5, 2023

This injects the code snippet as early in the file as possible, which is to say, after an initial block of "use …"; statements, if there is one. Since those statements must itself be at the start of the file (except for empty lines and comments), we skip empty lines and comments and then any number of "use …"; statements before injecting. For example, the file

// a
// comment
// block

// another
// comment
// block

'use strict';
"use server";
function t(t) {
  return '[object Object]' === Object.prototype.toString.call(t);
}
//# sourceMappingURL=/path/to/sourcemap

would be modified to

// a
// comment
// block

// another
// comment
// block

'use strict';
"use server";
<CODE_SNIPPET>[<debug_id>]
function t(t) {
  return '[object Object]' === Object.prototype.toString.call(t);
}
//# debugId=<debug_id>
//# sourceMappingURL=/path/to/sourcemap

Modifying the minified source file in this way requires us to fiddle with the sourcemap as well since every line number below the insertion point has now increased by one. Thus, we simply add ; at the start of the sourcemap's mapping string to denote the code snippet's empty line mapping.

Question: Is it sound to just always add the empty mapping at the beginning? If any of the lines before the injection point had mappings, they would get scrambled by this change, but since they're only comments and "use …";, can they even have nontrivial mappings?

@loewenheim
Copy link
Contributor Author

loewenheim commented Apr 11, 2023

As it turns out, this proposal has a serious flaw. Injecting text at the beginning of a file will throw off all offsets in the sourcemap. To make this work, we'd have to fix those up after the fact.

Edit: sourcemaps are now getting adjusted as required.

@ashwoods ashwoods marked this pull request as draft April 17, 2023 20:27
@github-actions
Copy link

github-actions bot commented May 9, 2023

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@loewenheim
Copy link
Contributor Author

Picking this up again because it turned out to be necessary in an internal test.

@loewenheim loewenheim marked this pull request as ready for review May 16, 2023 09:50
@kamilogorek
Copy link
Contributor

Looks good to me overall.

@loewenheim
Copy link
Contributor Author

One thing I definitely still need to handle is embedded sourcemaps.

@kamilogorek
Copy link
Contributor

TODO: Handle when no sourcemap file is available locally. In this case, we should append at the end of the file instead, as sourcemap cannot have its mappings fixed up.

@loewenheim
Copy link
Contributor Author

This now also handles the edge case of a sourcemap not being available, by essentially falling back to the status quo behavior. I also refactored inject_debug_ids some; the double loop is no longer necessary.

@loewenheim loewenheim merged commit 9f4df13 into master May 24, 2023
16 checks passed
@loewenheim loewenheim deleted the ref/inject-start branch May 24, 2023 09:29
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 inject: Inject code snippet at the beginning of the minified source file
2 participants