Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

changing imported css will be ignored in chunk hash #1660

Open
dionysiusmarquis opened this issue Dec 7, 2020 · 8 comments
Open

changing imported css will be ignored in chunk hash #1660

dionysiusmarquis opened this issue Dec 7, 2020 · 8 comments

Comments

@dionysiusmarquis
Copy link

Describe the bug
When you are changing the contents of an imported css in dev, you will see the changes immediately, but after a split second the page will be reloaded with the previous version. This is most likely because the js files file name (e.g. my-component-{hash}.js) will remain the same even though the corresponding my-component-{hash}.css changed. My guess is that the css file gets updated correctly, which will be applied immediately, but then a (unnecessary) page reload get's triggered. Because the js files hash is still the same even though the css contents changed (maybe because of the inject_css stuff happening on top of the rollup build pipeline?), the service worker will provide an old, cached version of that js file.

To Reproduce

// _layout.svelte
<script>
  import 'src/styles/test.css';
</script>

In Browser dev tools:

// client.cbe62f53.js
const components = [
  {
    js: () => Promise.all([, __inject_styles(["client-f2a00d8f.css"])]).then(function(x) { return x[0]; })
  },
  

after changing src/styles/test.css contents and "automated" page reload (wrong, still same css file hash)

// client.cbe62f53.js
const components = [
  {
    js: () => Promise.all([, __inject_styles(["client-f2a00d8f.css"])]).then(function(x) { return x[0]; })
  },
  

After "manual" page reload without any further css content change (correct css file hash)

// client.cbe62f53.js
const components = [
  {
    js: () => Promise.all([, __inject_styles(["client-d6af0d49.css"])]).then(function(x) { return x[0]; })
  },
  

Expected behavior
I think it would already be resolved if the page wouldn't reload on imported css change. The Service worker cache would be "out of sync" but that shouldn't have any side effects, I guess.

Information about your Sapper Installation:

  Binaries:
    Node: 14.15.1 - ~/.nvm/versions/node/v14.15.1/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.8 - ~/.nvm/versions/node/v14.15.1/bin/npm
  Browsers:
    Chrome: 87.0.4280.88
    Safari: 14.0.1
  npmPackages:
    rollup: ^2.3.4 => 2.34.0 
    sapper: ^0.28.0 => 0.28.10 
    svelte: ^3.17.3 => 3.30.1 

Severity
It's annoying because you always have to manually reload the page after the unnecessary "automated" page reload

@dionysiusmarquis
Copy link
Author

dionysiusmarquis commented Dec 7, 2020

After some more investigation it's not a "unnecessary" reload it's actually: Reload -> correct current version for a split second -> replaced by old cached version
It seems more like a SSR -> dev client mismatch

@dionysiusmarquis
Copy link
Author

dionysiusmarquis commented Dec 8, 2020

Making a "hard" reload will always work, but that just ignores "caching environments" which are the crucial part of this issue, as far as I can tell.

The main problem is most likely this:
https://github.com/sveltejs/sapper/blob/master/src/core/create_compilers/RollupCompiler.ts#L207

chunk.code = 

As this is done in a rollup bundle hook and not build hook the chunks hash will always remain the same even if you change complete contents of imported css files or any other part regarding css imports. This is pretty bad tbh. That means that caching prod environments won't apply css changes unless you also manually change at least anything in the contents of the chunks source.
I already tried to update the hash based on the imported css files in the current sapper bundle hooks, but unfortunately that is "too late" as all associated imports are already using the chunk file name coming from the build hooks.

@dionysiusmarquis dionysiusmarquis changed the title changing imported css will lead to unnecessary dev reload changing imported css will be ignored in chunk hash Dec 8, 2020
@dionysiusmarquis
Copy link
Author

Alright there is this augmentChunkHash hook. With that I managed to additionally apply the corresponding css hashes to the chunk hashes before the bundle hooks are kicking in. You will need to additionally load the css file contents, though.

In my current case with the all imported css related contents and hashes already available:

import { createHash } from 'crypto'

return {
  name: 'my-css-plugin',
  augmentChunkHash (chunk) {
      let hash
      for (const moduleId in chunk.modules) {
        const css = cssCache[moduleId]
        if (css) {
          if (!hash) {
            hash = createHash('sha256')
          }
          hash.update(css.hash)
        }
      }
      if (hash) {
        return hash.digest('hex')
      }
    },

@benmccann
Copy link
Member

@dionysiusmarquis thanks for digging into this. I probably won't have time to work on this, but if you'd like to send a PR I'd be happy to review and merge

@dionysiusmarquis
Copy link
Author

dionysiusmarquis commented Dec 9, 2020

I created a PR. I also tried to add support for additional "css extensions" like .pcss, .sass… But rollup-plugin-css-chunkswill only react to .css. I also tried to use rollup-plugin-postcss, but Unfortunately the plugin will change all chunk hashes on any css related change atm…
For my use case it's "ok" since I only want to apply postcss - and using .css here is pretty valid, I guess. So to break it down: If you want to bypass svelte handling your processed css, it is currently still a bit messy, if you try to solve it the context of sapper. With SvelteKit on the horizon I guess I'm happy for now 😛

@benmccann
Copy link
Member

I just tried to reproduce this against master and was not able to. Can you check if it reproduces for you on master? I'm wondering if perhaps it's already been fixed somehow by another change. If you can reproduce it, can you share a sample project for reproducing?

@benmccann
Copy link
Member

@dionysiusmarquis not sure if you saw my last comment, but I'm wondering how to reproduce this. And also the PR will need to be rebased. Hope you had some happy holidays!

@dionysiusmarquis
Copy link
Author

dionysiusmarquis commented Jan 25, 2021

It's still present on sapper 0.28.10. I also tested 2.29.0 real quick in dev, but in this version client.css won't be hashed at all.

I can still reproduce it quite easily on a fresh sapper setup:
Add a css file e.g. src/css/test.css

body {
  color: green;
}

import the file inside _layout.svelte

<script lang="ts">
  import '../css/test.css';
  import Nav from '../components/Nav.svelte';

  export let segment: string;
</script>

Change src/css/test.css contents e.g.

body {
  color: red;
}

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

Successfully merging a pull request may close this issue.

2 participants