Skip to content

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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cprecioso
Copy link

@cprecioso cprecioso commented Jun 7, 2022

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
Copy link

changeset-bot bot commented Jun 7, 2022

🦋 Changeset detected

Latest commit: 6ab8a0a

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

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.

1 participant