-
Notifications
You must be signed in to change notification settings - Fork 124
Description
Related to #304
The latest version of importmap has broken the bin/importmap pin command in its default state:
bruno@SurfaceBruno:~/app$ bin/importmap pin @github/template-parts
Pinning "@github/template-parts" to vendor/javascript/@github/template-parts.js via download from https://ga.jspm.io/npm:@github/[email protected]/lib/index.js
Using integrity: sha384-lZhYSBnI5wsFksTQATVO5gqaU6AnipVxNXk6F6QN6VGxzTwCEtcHzcLvc5OOhzYT
When importing the package I get the following in the browser console:
Failed to find a valid digest in the 'integrity' attribute for resource 'http://localhost:3000/assets/@github--template-parts-c3786bf8.js' with computed SHA-384 integrity 't7jeOzC6siiidZ7TosBmFCMpEmIAMIi7ydGfuDz+J5zkZdud/GdvXQwrkp1ZIwSg'. The resource has been blocked.
I don't really understand the intention behind this feature in this context. It seems to be using JSPM integrity, but the package is on local, since when you pin it, it is always downloaded and never requested from the CDN in the browser, so that integrity is not correct.
Downloading the package in importmap is now the only option for the bin/importmap pin
command, so I don't see much point in this. You could manually include integrity in case you have packages pointing to CDN links in the configuration, but that's not what the command does when you pin a package; it just downloads it and that's it.
Similarly, this command does not work either:
bruno@SurfaceBruno:~/app$ bin/importmap integrity
Couldn't find any packages in ["@github/[email protected]"] on jspm
Even so, the package is in the vendor folder, so it doesn't make sense to get the integrity from JSPM.
If I use the same integrity and pin it to the CDN link it works:
pin "@github/template-parts", to: "https://ga.jspm.io/npm:@github/[email protected]/lib/index.js", integrity: "sha384-lZhYSBnI5wsFksTQATVO5gqaU6AnipVxNXk6F6QN6VGxzTwCEtcHzcLvc5OOhzYT" # @0.7.1
But the bin/importmap integrity command doesn't work anyway:
bruno@SurfaceBruno:~/app$ bin/importmap integrity
Couldn't find any packages in ["@github/[email protected]/lib/index.js\", integrity: \"sha384-lZhYSBnI5wsFksTQATVO5gqaU6AnipVxNXk6F6QN6VGxzTwCEtcHzcLvc5OOhzYT\" # @0.7.1", "@github/[email protected]"] on jspm
Then, I removed the package integrity and set it to true, and it should calculate it, but that doesn't happen either:
pin "@github/template-parts", to: "@github--template-parts.js", integrity: true # @0.7.1
bruno@SurfaceBruno:~/app$ bin/importmap json
{
"imports": {
"application": "/assets/application-1d706dac.js",
"@hotwired/turbo-rails": "/assets/turbo.min-3a2e143f.js",
"@hotwired/stimulus": "/assets/stimulus.min-4b1e420e.js",
"@hotwired/stimulus-loading": "/assets/stimulus-loading-1fc53fe7.js",
"@github/template-parts": "/assets/@github--template-parts-c3786bf8.js",
"controllers/application": "/assets/controllers/application-3affb389.js",
"controllers/hello_controller": "/assets/controllers/hello_controller-708796bd.js",
"controllers": "/assets/controllers/index-ee64e1f1.js"
}
}
This is because config.assets.integrity_hash_algorithm
is nil and therefore never calls the method to return the asset's integrity. However, this is not very clear at first glance. You have to read the Propshaft readme to know that you have to configure the algorithm manually, and then that will allow Propshaft to return the integrity. I discovered this by debugging instead of reading the Propshaft readme, actually.
I finally left integrity enabled for all packages, but I had to set the algorithm and write integrity: true
on each line. Wouldn't it be better if this were ready out of the box?
Just as preload is true by default, if the hash algorithm is set, which I assume should also be set to sha384 by default, then I also think this should be enabled by default. That way, you don't have to write integrity: true
so many times, which is quite redundant.
But I would remove everything related to integrity from the CDNs. I would just leave the option to add it manually. No command will leave the packages pined from the CDN, so I don't see the point.
@rafaelfranca, what do you think? I can help with this if it's useful, but as it stands, this is broken for any new user And it's not very intuitive at all.