fix(rollup-plugin-import-meta-assets): completely overwrite import.meta.url

[cprecioso]

This removes the duplicated import.meta.url in the output, like this:

 // Pre-bundling code
 new URL("./some-internal-path.png", import.meta.url)
 
 // Bundled code in ESM output 
-new URL(new URL("asset/bundled-asset.png", import.meta.url).href, import.meta.url) 
+new URL(new URL("asset/bundled-asset.png", import.meta.url).href) 

 // Bundled code in CJS output 
 new URL(
   "undefined" == typeof document
     ? new (require("url").URL)(
         "file:" + __dirname + "/assets/InterLatin-b0e9bc2a.woff2"
       ).href
     : new URL(
         "assets/InterLatin-b0e9bc2a.woff2",
         (document.currentScript && document.currentScript.src) ||
           document.baseURI
       ).href
-, import.meta.url); // Uncaught SyntaxError: Cannot use 'import.meta' outside a module
+);

Why

This way, it should work exactly the same (Rollup makes sure we have import.meta.url or something similar to base our URL on); but the second import.meta gets eliminated, which reduces size, repetition, and most of all, doesn't trigger an error in Common JS modules.

What I did

1. Updated the overwrite function

   // Before
   new URL("myFile.tx", import.meta.url)
   //      |---------|
   //      Only this part was overwritten
   
   // After
   new URL("myFile.tx", import.meta.url)
   //      |--------------------------|
   //      Now everything is overwritten

2. Updated tests

I made it a minor change as it should supposedly not modify the functionality of the plugin, but maybe it should be a major as it modifies its output?

[changeset-bot]

🦋 Changeset detected

Latest commit: 6a50cd7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@web/rollup-plugin-import-meta-assets	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[cprecioso]

Closing due to old age

[bashmish]

Hey, it's actually a great change, too bad I didn't see it before today. I can sync with latest master, I think many tests have changed since this was created. But if you still wanna work on this, I'm happy to assist. Let me know what works best for you!

[cprecioso]

@bashmish Synced and fixed tests

[bashmish]

maybe I mislooked first time, but I was under impression that the whole outer URL wrapper will be gone with you change, not only the import.meta.url
do you think it's a more complex change or is the outer one still needed?

[cprecioso]

The outer one is still needed for the most general case.

When using the import.meta.ROLLUP_FILE_URL_referenceId notation, Rollup will generate an expression of type string, not of type URL. The expression does use URL internally but it's an implementation detail (that changes depending on the output format), and the final result is still a string. So to get a URL as the new URL("./asset", import.meta.url) expression suggests, we need to wrap it inside a new URL() call anyway. You can see example output for both ESM and CJS in the first code example in the PR description.

The main problem I was trying to solve with this PR is not removing URL constructors, but the fact that keeping the outermost import.meta.url was tripping up CJS support.

[cprecioso]

There's an optimization that you can do if the input is new URL("./asset", import.meta.url).href, as you could replace that whole-sale with the import.meta.ROLLUP_FILE_URL_referenceId expression and be correct, but only for the case where we're calling .href or .toString(). I do not have the bandwidth to implement that currently.

[bashmish]

ok got it! thanks for the explainer, helps with the decision making a lot and historical reference