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

An external img should not generate an import statement #8047

Closed
7 tasks done
bseib opened this issue May 6, 2022 · 10 comments
Closed
7 tasks done

An external img should not generate an import statement #8047

bseib opened this issue May 6, 2022 · 10 comments

Comments

@bseib
Copy link

bseib commented May 6, 2022

Describe the bug

Here's the setup. Page1.vue contains an external image that doesn't exist in this repo**:

<img alt="smpte bars" class="logo" src="/img/aaaSMPTE-color-bars.png" />

There's a entry-point-1.ts file which includes the Page1.vue file, and that entry point file is specified as an
input in the vite.config.ts config. The vite.config.ts also has a rollup config that specifies that PNG to be
external:

export default defineConfig({
  // ...
  build: {
    rollupOptions: {
      input: [
        'src/entry-point-1.ts',
      ],
      external: [
        '/img/aaaSMPTE-color-bars.png',
      ],
      // ...
    }
  },
  // ...
})

Expected Behavior

Run npm run build.

After running npm run build, the generated .js file should not attempt to import anything for that <img> source, and the string literal /img/aaaSMPTE-color-bars.png should be left intact when rendering that <img>. That <img> is expected to be completely external (honoring the rollup config), and should be ignored by any client code.

Actual Behavior

The build does not ignore that <img> tag, and generates an import statement that expects that image file to be in a relative
location, i.e. in the build output,

File dist/assets/entry-point-1.e49989cd.js:

[blahblahblah]import n from"../../img/aaaSMPTE-color-bars.png";[blahblahblah]()=>p("img",{alt:"smpte bars",class:"logo",src:n},null,-1)

That import breaks at runtime on the client side. That import should not exist in the build output, and the rendering of the "img" tag should be a static string value rather than the imported variable (n).

Reproduction

https://github.com/bseib/vue-vite-external-static-image

System Info

$ npx envinfo --system --npmPackages '{vite,@vitejs/*}' --binaries --browsers

  System:
    OS: Windows 10 10.0.19042
    CPU: (8) x64 Intel(R) Core(TM) i7-3770 CPU @ 3.40GHz
    Memory: 4.69 GB / 15.81 GB
  Binaries:
    Node: 14.15.4 - C:\Program Files (x86)\Nodist\bin\node.EXE
    Yarn: 1.22.5 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.14.10 - C:\Program Files (x86)\Nodist\bin\npm.EXE
  Browsers:
    Chrome: 100.0.4896.127
    Edge: Spartan (44.19041.1266.0), Chromium (101.0.1210.39)
    Internet Explorer: 11.0.19041.1566

Used Package Manager

npm

Logs

No response

Validations

@bseib
Copy link
Author

bseib commented May 6, 2022

To reproduce the issue:

git clone git@github.com:bseib/vue-vite-external-static-image.git
cd vue-vite-external-static-image
npm install && npm run build

Or here's the same code running on stackblitz that can demonstrate the issue:
https://stackblitz.com/edit/vitejs-vite-t5npgf?file=dist%2Fassets%2Fentry-point-1.ede3131f.js&terminal=dev

You can see the erroneous output in the entry-point-1.ede3131f.js file on StackBlitz, but might be easier to follow along and see the screenshot in the github issue linked above.

image

There are more details in the github repo link, and a broken.html file you can use to actually see this fail. I tried to document the things I tried in the README of that repo. I couldn't get StackBlitz to serve up the broken.html failure, since it is a product of build-time, whereas stackblitz is really good at dev-time demonstrations. But you can follow the instructions in the github example to see it break.

@bseib
Copy link
Author

bseib commented May 6, 2022

Thanks @LinusBorg for getting my post into the correct repo. Thorsten already did some homework, which I am copy/pasting from the other place where I had opened the issue.

This is pretty certainly a problem with vite, maybe more specifically its vue plugin (also maintained in the vite repo).

Noteworthy to mention when moving you issue there:

  • the issue, in its reported form, happens because you added the path to rollupOptions.external
  • however when you don't do that, and the file is not present in /public under that path, you get an error reported that rollup can't resolve an import for that path - and get a recommendation to add it to rollupOptions.external, which is likely how you end up where you did.

But our template compiler leaves absolute paths untouched:

image

See JS tab on the right side of this playground

So my non-expert opinion is that Vite somehow stille extracts this path, makes it part of its module graph and tries to resolve its existance (likely because it also does that during dev to be able to serve it? rollup/vite should just ignore this path during build, or maybe issue a warning to make you aware the file was not found in /public (where it's supposed to be found normally).

TBH I could be really wrong in my reasoning as to why vite fails here but at the end of the day, in one way or another this is to be solved in vite / @vitejs/plugin-vue IMHO


Here's the culprit:

} else {
// build: force all asset urls into import requests so that they go through
// the assets plugin for asset registration
assetUrlOptions = {
includeAbsolute: true
}
}

this option is forced to be set and is responsible for absolute paths to be part of the asset pipeline.

Please move this to the vite repo, thanks!

@bseib
Copy link
Author

bseib commented May 6, 2022

Also, looking through other issues here, it's possible that #6931 and #3533 could be related, although I am not certain.

@poyoho
Copy link
Member

poyoho commented May 10, 2022

config.assetsInclude's assets load error, but I think this is expect? because vue make the asset import like

import noExistImg from "/img/aaaSMPTE-color-bars.png"

if /img/aaaSMPTE-color-bars.png no exist I think call error is better. emm... but now it seems to no way to ignore this error.

@bluwy
Copy link
Member

bluwy commented May 15, 2022

Here's the culprit:

} else {
// build: force all asset urls into import requests so that they go through
// the assets plugin for asset registration
assetUrlOptions = {
includeAbsolute: true
}
}

this option is forced to be set and is responsible for absolute paths to be part of the asset pipeline.

Please move this to the vite repo, thanks!

This is fixed in #6779, so if you specify template.transformAssetUrls.includeAbsolute: false to the vue plugin, this issue can be workaround.

@bseib
Copy link
Author

bseib commented May 22, 2022

Thanks @bluwy, I just tried out this workaround (in vite v2.9.8). But I didn't get it to work. Did I specify template.transformAssetUrls.includeAbsolute: false correctly?

Here's what I changed in the vite configs:

diff --git a/vite.config.ts b/vite.config.ts
index 7a073c5..8535119 100644
--- a/vite.config.ts
+++ b/vite.config.ts
@@ -18,7 +18,13 @@ export default defineConfig({
       ],
     }
   },
-  plugins: [vue()],
+  plugins: [vue({
+    template: {
+      transformAssetUrls: {
+        includeAbsolute: false
+      }
+    }
+  })],
   // resolve: {
   //   alias: {
   //     '@': fileURLToPath(new URL('./src', import.meta.url))

(Or see the entire vite.config.ts file in my workaround branch.)

I get the exact same output with and without the workaround in the configs. Either way, running npm run build produces the exact same output. The file dist/assets/entry-point-1.e49989cd.js still contains an unwanted import:

[blahblahblah]import n from"../../img/aaaSMPTE-color-bars.png";[blahblahblah]()=>p("img",{alt:"smpte bars",class:"logo",src:n},null,-1)

Did I apply the workaround incorrectly?

@LinusBorg
Copy link
Collaborator

I think that you should not add these images to rollupOptions.external

@bseib
Copy link
Author

bseib commented May 22, 2022

Okay, I gave that a shot:

vite.config.ts:

import { fileURLToPath, URL } from 'url'

import { defineConfig } from 'vite'
import vue from '@vitejs/plugin-vue'

// https://vitejs.dev/config/
export default defineConfig({
  build: {
    manifest: true,
    rollupOptions: {
      input: [
        'src/entry-point-1.ts',
        'src/entry-point-2.ts',
      ],
      external: [
      ],
    }
  },
  plugins: [vue({
    template: {
      transformAssetUrls: {
        includeAbsolute: false
      }
    }
  })],
})

But now I'm back to Rollup being unhappy with me:

$ npm run build
npm WARN lifecycle The node binary used for scripts is C:\Program Files (x86)\Nodist\bin\node.exe but npm is using C:\Program Files
(x86)\Nodist\v-x64\14.15.4\node.exe itself. Use the `--scripts-prepend-node-path` option to include the path for the node binary npm
 was executed with.

> vue-vite-external-static-image@0.0.0 build C:\git\vue-vite-external-static-image
> vue-tsc --noEmit && vite build

vite v2.9.8 building for production...
✓ 8 modules transformed.
[vite]: Rollup failed to resolve import "/img/aaaSMPTE-color-bars.png" from "src/Page1.vue".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`
error during build:
Error: [vite]: Rollup failed to resolve import "/img/aaaSMPTE-color-bars.png" from "src/Page1.vue".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`
    at onRollupWarning (C:\git\vue-vite-external-static-image\node_modules\vite\dist\node\chunks\dep-e1fc1d62.js:41550:19)
    at onwarn (C:\git\vue-vite-external-static-image\node_modules\vite\dist\node\chunks\dep-e1fc1d62.js:41366:13)
    at Object.onwarn (C:\git\vue-vite-external-static-image\node_modules\rollup\dist\shared\rollup.js:23184:13)
    at ModuleLoader.handleResolveId (C:\git\vue-vite-external-static-image\node_modules\rollup\dist\shared\rollup.js:22474:26)
    at C:\git\vue-vite-external-static-image\node_modules\rollup\dist\shared\rollup.js:22435:26
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! vue-vite-external-static-image@0.0.0 build: `vue-tsc --noEmit && vite build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the vue-vite-external-static-image@0.0.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

@LinusBorg
Copy link
Collaborator

LinusBorg commented May 22, 2022

did you update @vitejs/plugin-vue (2.3.2), as that's where the fix happened?

@bseib
Copy link
Author

bseib commented May 22, 2022

Whoops! I did not update the @vitejs/plugin-vue, despite acknowledging that I needed to! 😳 🤦‍♂️

Using that workaround, the output looks correct now compared to the previous output, with the excess import statement removed:

image

I pushed a branch showing this working.

Thanks @LinusBorg and @bluwy .

@github-actions github-actions bot locked and limited conversation to collaborators Jun 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants