stencil icon indicating copy to clipboard operation
stencil copied to clipboard

"p-*"-files are not immutable. Hash-filename doesn't update when content changes.

Open mikkelrom opened this issue 4 years ago • 26 comments

Stencil version:

 @stencil/[email protected]

I'm submitting a:

[x] bug report [ ] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior:

Stencil-config setting: hashFileNames not working entirely as expected. p-*-files are not immutable even though they should be, since the content within can change without the filename itself changes.

Expected behavior:

p-*-filenames should change if the content within changes, so those files can be "forever-cached", which they cannot at the moment even though the docs says:

During production builds, the content of each generated file is hashed to represent the content, and the hashed value is used as the filename. If the content isn't updated between builds, then it receives the same filename. When the content is updated, then the filename is different. By doing this, deployed apps can "forever-cache" the build directory and take full advantage of content delivery networks (CDNs) and heavily caching files for faster apps."

Steps to reproduce:

https://github.com/mikkelrom/stencil-caching-issue

Other information:

Related issue: https://github.com/ionic-team/stencil/issues/1983

Information about rollup issue

The team behind http://web.dev created this to keep track of bundlers and their errors. This failing test in Rollup seems to maybe be causing this issue: https://bundlers.tooling.report/hashing/asset-cascade/ Rollup issue: https://github.com/rollup/rollup/issues/3415

mikkelrom avatar Jun 22 '20 11:06 mikkelrom

Update: It looks like Vite has made a workaround for the Rollup issue, maybe this could be implemented in Stencil as well?

https://github.com/vitejs/vite/commit/f8e4eebcc48ad5e98e52d12ce6595da014a43276

mikkelrom avatar Jul 12 '21 13:07 mikkelrom

Hey @mikkelrom - I tagged this so Ryan can confirm this bug and go from there.

splitinfinities avatar Sep 30 '21 17:09 splitinfinities

Thank you @splitinfinities, sounds great 🙌 🙂

mikkelrom avatar Sep 30 '21 17:09 mikkelrom

Hi, is there an update on this? I'm running into the same issue.

Basically, my build produces my-library.js and a bunch of p-.system.js . When I deploy this, everything works fine. On a second build and deploy, my-library.js calls te same file, say p-123.system.js like the last time, but p-123 has now changed and calls different p-files. Problem is, p-123 is cached and keeps calling files that don't exist anymore. Is there a way to force it so that all filenames change on every build?

janarvaez avatar Feb 02 '22 09:02 janarvaez

I even tried setting the hashedFileNameLength to 12, and some files stay at 8 and some at 12.

janarvaez avatar Feb 02 '22 09:02 janarvaez

@splitinfinities @rwaskiewicz any update on this? 🙂

mikkelrom avatar Feb 10 '22 10:02 mikkelrom

This seems like a potentially huge foundational issue for Stencil (and Ionic) and it is a little shocking to see so little official response for something that has been lingering since being reported in 2019.

I know you guys are busy and it is annoying to get lots of issues and feature requests so it is easy for things to get lost in the list. But this one seems important. If you can not reproduce the problem, don't think it is a bug, are waiting on some other fix, have a suggested workaround, or even just agree that this is an important thing to fix that would be great to know.

There are cases where getting the wrong version of a file can have a minor impact (like stale content) but there are also cases where the wrong version of a file can totally break an application. For example, missing import files break <ion-nav> (https://github.com/ionic-team/ionic-framework/issues/21046). Further details on the impact of missing files is also in https://github.com/ionic-team/stencil/issues/2376. Workarounds like refreshing the page, CDN expirations, etc. do not necessarily fix the problem and it seems like something that can only be handled during the build process.

If it would be helpful for me to provide any more details about the downstream impact of non-immutable files then let me know and I can provide more detail.

It is similar to if npm install sometimes resulted in incorrect versions of some of the files.

cjorasch avatar Feb 10 '22 16:02 cjorasch

hey @mikkelrom - I took the liberty of taking your reproduction repo for a spin, but could not recreate the issue using the steps described: "Hello world!" hello world! "Hello world 2!" hello world 2!

Further to this, I created a sandbox which isolates the snippet stencil is using under the hood for file hashes here and stencil seems to be consistent... If someone can create a reliable reproduction, I'll take a look.

Since the file hashing is done by node's internal 'crypto' module, it could boil down to the version of node used? ... I realise I'm just grasping

johnjenkins avatar Feb 11 '22 10:02 johnjenkins

Hi @johnjenkins. Thanks for taking the time to look at this (to me, critical) issue and the reproduction repo 👍 🙏 I've just pulled down the repo again and followed the steps, and I still get the same behaviour as described in the issue. I'm using node version 12.22.1.

As mentioned, I'm pretty convinced that the issue is caused by Rollup. Please see here: https://bundlers.tooling.report/hashing/asset-cascade/ and here: https://github.com/rollup/rollup/issues/3415

And as I mentioned in the first comment:

It looks like Vite has made a workaround for the Rollup issue, maybe this could be implemented in Stencil as well? https://github.com/vitejs/vite/commit/f8e4eebcc48ad5e98e52d12ce6595da014a43276

mikkelrom avatar Feb 11 '22 15:02 mikkelrom

It's not clear to me how https://bundlers.tooling.report/hashing/asset-cascade/ is relevant because in this instance it's the actual 'core' file who's source is changing, not an imported asset.

And I also don't think it's to do with rollup because rollup actually doesn't control the file hashing - that step happens after the rollup bundle, (it happens here < using 'rollupResults' above) just before the files are written to disk.

Interestingly the vite fix you note here Is very similar to what stencil is already doing here < except using sha256 instead of sha1.

So I suppose it comes down to local environment. Using stencil v2.13, what do you get when running stencil info?

Also, have you tried enableCache: false, in your stencil.config?

johnjenkins avatar Feb 11 '22 16:02 johnjenkins

Hi again @johnjenkins, sorry for the late reply.

This issue was created a long time ago, so I decided to try and update the reproduction repo to the latest version of Stencil, as you suggested (from v1.12.2 to v2.13.0). And after updating I can't seem to reproduce the issue, which is really great! 🎉 The filename seem to change when the contents of the file is changed, which is great 🙂

I'm curious to know if this issue is also solved for others, just by using latest Stencil version? @janarvaez @cjorasch ?

mikkelrom avatar Feb 14 '22 07:02 mikkelrom

Hi again @johnjenkins, sorry for the late reply.

This issue was created a long time ago, so I decided to try and update the reproduction repo to the latest version of Stencil, as you suggested (from v1.12.2 to v2.13.0). And after updating I can't seem to reproduce the issue, which is really great! 🎉 The filename seem to change when the contents of the file is changed, which is great 🙂

I'm curious to know if this issue is also solved for others, just by using latest Stencil version? @janarvaez @cjorasch ?

Hi! I'm afraid I had recently upgraded at the time of my comment, which was 2.12.1, and was still encountering the issue. Can't share my repo as it's a company owned project. I'll do my best to try and create a minimal version that reproduces this.

janarvaez avatar Feb 14 '22 07:02 janarvaez

Hi @johnjenkins (cc @mikkelrom )

Here is a reproduction repo which can hopefully help:

https://github.com/janarvaez/stencil-build-issue

I left what I did in the README.md This one is using 2.12.1, and I tried to remove as much code as possible, so please forgive me if there's some noise or undesired code/files in there... I also forgot to gitignore the www and loader folders 😬

janarvaez avatar Feb 14 '22 09:02 janarvaez

Hi All! Just updated my project to stencil 2.14.0 and issue still there.

% stencil info
System: node 14.17.1
Plaform: darwin (19.6.0)
Compiler: .../node_modules/@stencil/core/compiler/stencil.js
Stencil: 2.14.0 💫
TypeScript: 4.5.4
Rollup: 2.42.3
Parse5: 6.0.1
Sizzle: 2.42.3
Terser: 5.6.1

bzolotukhyn avatar Feb 17 '22 07:02 bzolotukhyn

to be clear, @bzolotukhyn @cjorasch @janarvaez is the issue primarily with the .system.js modules (i.e. the es5 build)? Or do you see it other places too?

johnjenkins avatar Feb 17 '22 09:02 johnjenkins

@johnjenkins I compared the last 5 builds of my library and in my case only one file (the same) doesn't change name when content changes and yes it's .system.js file.

bzolotukhyn avatar Feb 17 '22 12:02 bzolotukhyn

Update: Rollup issue fixed in v3.0.0.

https://github.com/rollup/rollup/issues/3415

mikkelrom avatar Oct 11 '22 14:10 mikkelrom

Excellent - we're scheduling Rollup-related updates starting next sprint.

rwaskiewicz avatar Oct 11 '22 14:10 rwaskiewicz

Excellent - we're scheduling Rollup-related updates starting next sprint.

@rwaskiewicz any update on this and process on how to integrate it with existing stencil project?

durgaprasad95 avatar Jan 06 '23 12:01 durgaprasad95

Hey @durgaprasad95, just a quick update: we'll be sure to ping here when we have a pre-release version of Stencil with Rollup v3 to test and confirm that this issue is addressed.

Unfortunately upgrading Rollup in Stencil is quite involved because Stencil integrates very closely with Rollup, uses a lot of custom plugins, and so on. Because we're currently using a fairly outdated version we have to do something of a stepwise upgrade to eventually get up to v3.

All that is to say that while we can't make a promise as to a specific timeline at present, the team is working on it and it is a priority for us.

alicewriteswrongs avatar Jan 06 '23 15:01 alicewriteswrongs

@alicewriteswrongs is there any update on this issue?

john-traas avatar Sep 25 '23 11:09 john-traas

Hi @alicewriteswrongs and @rwaskiewicz

Thanks for being an active participant in the StencilJS. Wondering if there are any updates for this issue?

chiragajmeri avatar Sep 26 '23 20:09 chiragajmeri

Hey folks, we don't have any updates on this at this time. We put the rollup changes on hold for a few months, and will plan on picking them up again before the end of this year

rwaskiewicz avatar Sep 27 '23 21:09 rwaskiewicz

Thank you for the update @rwaskiewicz - much appreciated.

john-traas avatar Oct 05 '23 12:10 john-traas

I came up with a workaround, in case it helps anybody: to include a build timestamp prop in all components, which forces StencilJS to re-generate all files as the content always changes with every production build. Not ideal, but given the fact that the issue has been open for 4 years I don't expect any updates on it soon.

As an idea to solve the issue in the core, instead of going down the rabbit hole of upgrading Rollup: this could be easily solved if StencilJS offered a method to pass a "?v" query string to the lazy-loader, in the same way that we set the setNonce() when loading the components. That way we ensure that all lazy-loaded files are appended with a version string on the URL, which will ensure the CDN always fetches the requested version of the file.

jespejoh avatar Mar 21 '24 12:03 jespejoh

Insufficient rollup version: "@rollup/plugin-commonjs" requires at least [email protected] but found [email protected]. (plugin: commonjs, buildStart)

i am getting this issue below are my versions

yarn list --pattern rollup yarn list v1.22.22 ├─ @rollup/[email protected] ├─ @rollup/[email protected] ├─ [email protected] ├─ [email protected] ├─ [email protected] └─ [email protected] Done in 0.28s.

akshaybachhav avatar Sep 11 '24 06:09 akshaybachhav