Does not appear to work in Vite

aral commented 3 years ago

Hi there,

First off, thank you for making and sharing this :)

I’ve been trying to get the plugin to work with Vite but it doesn’t seem to have an affect.

Thoughts? Should I open this issue in Vite instead?

To reproduce

npm init @vitejs/app
Name it vite-sri-test and choose vanilla for template to create the simplest possible app
cd vite-sri-test
npm i
npm run build and note the index.html file output in dist/:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <link rel="icon" type="image/svg+xml" href="/assets/favicon.17e50649.svg" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Vite App</title>
  <script type="module" crossorigin src="/assets/index.b2293eba.js"></script>
  <link rel="stylesheet" href="/assets/index.ccce2ca3.css">
</head>
  <body>
    <div id="app"></div>

  </body>
</html>

npm i rollup-plugin-sri
Create vite.config.js with the following content:

import sri from 'rollup-plugin-sri'
import { defineConfig } from 'vite'

export default defineConfig({
  plugins: [sri()]
})

npm run build and note the index.html file output in dist/:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <link rel="icon" type="image/svg+xml" href="/assets/favicon.17e50649.svg" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Vite App</title>
  <script type="module" crossorigin src="/assets/index.b2293eba.js"></script>
  <link rel="stylesheet" href="/assets/index.ccce2ca3.css">
</head>
  <body>
    <div id="app"></div>

  </body>
</html>

Add the @rollup/plugin-html and try with that (npm i @rollup/plugin-html + add the import line (import html from '@rollup/plugin-html' and change plugins: [sri()] to plugins: [html(), sri()) and try npm run build again. Note the output:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <link rel="icon" type="image/svg+xml" href="/assets/favicon.17e50649.svg" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Vite App</title>
  <script type="module" crossorigin src="/assets/index.b2293eba.js"></script>
  <link rel="stylesheet" href="/assets/index.ccce2ca3.css">
</head>
  <body>
    <div id="app"></div>

  </body>
</html>

Add the plugins to build.rollupOptions.plugins instead and note the resulting HTML (this time it’s different but still doesn’t include the integrity hashes):

<!doctype html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>Rollup Bundle</title>
    <link href="assets/index.ccce2ca3.css" rel="stylesheet">
  </head>
  <body>
    <script src="assets/index.b2293eba.js" type="module"></script>
  </body>
</html>

What should happen

Integrity hashes should be included

What actually happens

Integrity hashes are not included

More details

This output from Vite could be related:

rendering chunks (1)...The emitted file "index.html" overwrites a previously emitted file of the same name.

Version of Vite: 2.1.4 Version of rollup-plugin-sri: 1.2.7 Version of @rollup/plugin-html: 0.2.3

JonasKruckenberg commented 3 years ago

Thanks for the heads up! The TLDR version is: This is a Vite particularity, but a feature not a bug. The long version is as follows: Back when I wrote this plugin, the only way to get the full content of each asset (even those generated by other plugins) was to read them back from disk in the writeBundle hook. This is not elegant but it works in rollup. In Vite however, the writeBundle hook never get's called (A Vite particularity) another problem is that the index.html gets treated differently than other assets and there is a separate hook for that file.

So to recap, I didn't expect the plugin to work out of the box, but since Vite seems to be a popular choice and because I'm a lot more familiar with Vite now, I might revisit this plugin and see if I can add Vite support 👍

JonasKruckenberg commented 3 years ago

I've seen you forked the repo, If you already found a solution to make this work with Vite, feel free to submit a PR!

aral commented 3 years ago

Hey @JonasKruckenberg, thanks for getting back to me. I’ve been looking into this and it looks like while Vite supports Rollup plugins, it also has its own extensions to them, including a specific hook for getting configuration options and for transforming the index html file.

It’s actually so different that I’ve started writing my own plugin specifically for Vite.

It wouldn’t be impossible to support both use cases with one plugin but – unless I’m mistaken – it would essentially be akin to maintaining two plugins.

If you’d like to keep this issue open for yourself and attempt that, please do but I’m going to crack on with this specific plugin. These bundling workflows are overly complicated as they are :)

JonasKruckenberg commented 3 years ago

Jep that seems to be a better option, after looking into the issue a bit more it seems the problem isthat vite puts assets into the assets folder without passing that info via “native” rollup hooks.

The solution would be to access the base attribute of the vite config but that'd add vite-specific code to a rollup plugin, so yeah.

I thinkfor documentation purposes this issue should be closed once your plugin goes live?

JonasKruckenberg commented 3 years ago

Update!

the problem is that vite puts assets into the assets folder without passing that info via “native” rollup hooks.

Nevermind actually! Line 64 seems to be the culprit, and just removing that seems to make the plugin work with Vite no problem! I honestly don't remember what that was for and I will definitely do some more investigation and compatibility tests, but it seems that this fixes it.

However, I'd be interested in your improvements to the idea maybe we can even merge those back into this project so we're bundling our forces.

Anyway, awesome issue by the way! I've not seen such a thorough bug report in a long time 👍

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 1.2.8 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

JonasKruckenberg commented 3 years ago

Yeah don't know, sematic-release is always a little too excited ;) I'm fairly confident the released version (1.2.8) works with Vite, but if you find the time I'd be awesome if you could confirm that it indeed works!

aral commented 3 years ago

@JonasKruckenberg Happy to hear it. Just tested and it works well for local paths. For remote URLs (in both scripts and stylesheets), I get the following errors:

ENOENT: no such file or directory, open '/home/aral/sandbox/vite-sri-test-with-rollup-plugin/dist/https:/ar.al/index.js'
ENOENT: no such file or directory, open '/home/aral/sandbox/vite-sri-test-with-rollup-plugin/dist/https:/ar.al/style.css'

Looks like it’s treating them like local paths.

(In the meanwhile, I did write a plugin specifically for Vite that does the same thing for index.html which I’ll probably be using myself as it’s in JS and easier to maintain for me. I’ll also be linking to yours in the comments/readme as its a much more generic solution.) :)

JonasKruckenberg commented 3 years ago

Thanks for the feedback! I didn't catch that during testing so it's great to hear.

I'm looking forwards to seeing your plugin as well and might link to yours too!

aral commented 3 years ago

Hey Jonas, it’s out: https://github.com/small-tech/vite-plugin-sri

Much less configurable and limited than yours and unashamedly inspired by it :) Thanks again for making this and sharing it + if you need me to test anything else, just shout.

JonasKruckenberg / rollup-plugin-sri