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

Make it easier to render script tags with SRI data #45

Closed
kumar303 opened this issue May 16, 2017 · 7 comments
Closed

Make it easier to render script tags with SRI data #45

kumar303 opened this issue May 16, 2017 · 7 comments

Comments

@kumar303
Copy link

When not using the html-webpack-plugin, one needs to write a fair amount of custom code to render a script tag with proper SRI attributes for each asset.

I'd like to propose a new saveAs option to make this easier. It would look like this:

import SriPlugin from 'webpack-subresource-integrity';

const plugins = [
    new SriPlugin({
        hashFuncNames: ['sha512'],
        saveAs: path.join(__dirname, 'dist', 'sri-mapping.json'),
    }),
];

It would write a JSON file that gives you all the data you need to construct script tags:

{
  "crossOriginLoading": "anonymous",
  "integrityValues": {
     "bundle1-3d9b377092c96e232575.js" : "sha512-3zYQGpoq6KJ4D...",
     "bundle2-385668e5e9c5eb1bbc7e.js" : "sha512-n8tGFPkzPLMGltk..."
  }
}

In any server view that writes an index page, one could then easily create a script tag like:

const baseName = filePath.split('/').pop();
return `<script  
  src="${filePath}" 
  integrity="${sriMapping.integrityValues[baseName]}" 
  crossOrigin="${sriMapping.crossOriginLoading}"
/>`;

Would you accept a patch for this?

BTW, this interface is what I was using in sri-stats-webpack but that one doesn't support webpack 2 so I have to migrate. The approached worked well enough for a universal React app.

@jscheid
Copy link
Collaborator

jscheid commented May 16, 2017

Hi, many thanks for your suggestion.

What you're after can be done without any changes to the plugin, here's one possible
solution
.

I realise this ☝️ is five lines of configuration more than in your approach (or ten, once you add proper error handling), and I like the idea of making it easier or less verbose in a future version. However, it would have to wait a few weeks because I'm not keen to add new features before 1.0.0 is out.

I'm also not fully convinced that just dumping the data into a JSON file is the best way to go about this. It's certainly simple and generic, but it also feels a bit crude. There's probably other data to be shuffled from the Webpack build to the server, and so now it's the developer's responsibility to juggle a number of JSON files, pick a place for them to live etc.

Obviously that's not the end of the world, and I might be overthinking this. I'd just like to have some confidence there's nothing better we can do. Are you aware of any consensus or discussion around this question (in the "Universal Webpack community", for lack of a better term)?

Does the workaround help with your problem at hand, and we can let this simmer for a while?

@jscheid
Copy link
Collaborator

jscheid commented May 17, 2017

I wonder whether it would help if the SRI hashes were added to the result of stats.toJson(). It's not currently possible (as far as I'm aware) but @sokra mentioned to me once that adding this ability was on the webpack 2.x roadmap. I don't think it has happened yet but it would probably not take too much to get this feature added. Would that be useful at all in your specific situation @kumar303 and do you think it would be useful in general?

@kumar303
Copy link
Author

Hi, thanks for all the feedback.

I think the bottom line is that universal webpack is quite annoying. The pattern we're using is to render the index page with Express and thus we have to get some SRI data somehow. Maybe there is a better way.

Thanks for the workaround. I guess it won't really be too different if SriPlugin dumped a JSON file or if I add a custom plugin with a couple of lines to dump it. Feel free to close the issue.

I wonder whether it would help if the SRI hashes were added to the result of stats.toJson().

Do you mean so that I can just use the webpack-stats-plugin to dump a JSON file? Yeah, that seems like it would be helpful.

@jscheid
Copy link
Collaborator

jscheid commented May 17, 2017

I think the bottom line is that universal webpack is quite annoying.

Heh, nicely put.

Do you mean so that I can just use the webpack-stats-plugin to dump a JSON file? Yeah, that seems like it would be helpful.

Exactly... and that way we side-step a number of (small) issues. Let me talk to @sokra again to find out more about adding custom data to the stats. I'll keep this open for now as a reminder. Thanks again!

@jscheid
Copy link
Collaborator

jscheid commented Jun 20, 2017

Consider heading over to https://webpack.js.org/vote/ and voting for "Plugins for Stats - Allow plugins to modify stats and stat output".

@kumar303
Copy link
Author

Voted!

@jscheid
Copy link
Collaborator

jscheid commented Apr 3, 2018

Coincidentally (with #74) this just got approved a few hours ago: webpack/webpack#6702 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants