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

[NEXT-684] Fast-refresh for CSS files is not working in Firefox #43396

Closed
1 task done
lucasmotta opened this issue Nov 25, 2022 · 85 comments · Fixed by #48578 or #49534
Closed
1 task done

[NEXT-684] Fast-refresh for CSS files is not working in Firefox #43396

lucasmotta opened this issue Nov 25, 2022 · 85 comments · Fixed by #48578 or #49534
Assignees
Labels
area: app App directory (appDir: true) linear: next Confirmed issue that is tracked by the Next.js team.

Comments

@lucasmotta
Copy link

lucasmotta commented Nov 25, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

    Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.1.0: Sun Oct  9 20:15:09 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T6000
    Binaries:
      Node: 19.1.0
      npm: 8.19.3
      Yarn: 1.22.19
      pnpm: 7.14.2
    Relevant packages:
      next: 13.0.6-canary.1
      eslint-config-next: 13.0.5
      react: 18.2.0
      react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

So I just started a new next.js app using the v13.0.5 with the experimental app folder. I noticed that CSS changes are not correctly updated in the browser. The browser is notified of the change, but the styles are not applied - if I manually refresh the browser I can see the correct styles.

I also tried the canary version and the error still exists. Moving to v13.0.4 seems to work fine.

And I also tried with Tailwind following the beta documentation and also it doesn't work.

Expected Behavior

Updating styles should fast refresh and update the page with the new styles.

Link to reproduction - Issues with a link to complete (but minimal) reproduction code will be addressed faster

https://stackblitz.com/edit/vercel-next-js-viybne?file=app/global.css

To Reproduce

Open the global.css file, change the colors and save the file.

You can see it working with the 13.0.4:
https://stackblitz.com/edit/vercel-next-js-6sncvk?file=app/global.css

NEXT-684

@lucasmotta lucasmotta added the bug Issue was opened via the bug report template. label Nov 25, 2022
@Svish
Copy link

Svish commented Nov 27, 2022

Been trying to get started with a Next@13 + Sanity@v3 setup today. Things seemed to be working fine until I tried to change the Tailwind styling...

Can confirm that HMR for Tailwind with Next > 13.0.4 definitely does not work properly. More specifically, versions above 13.0.4 seems to break the JIT-compiler of Tailwind? That is, given the following `src/app/page.tsx file:

export default function Home() {
  return (
    <>
      <div className="text-3xl font-bold underline bg-indigo-500">
        Hello world!
      </div>
      <div className="text-3xl font-bold underline bg-indigo-700">
        Hello world!
      </div>
      <div className="text-3xl font-bold underline bg-indigo-700">
        Hello world!
      </div>
    </>
  )
}

If I change the classes and save the file, the browser does update immediately, but I change to a previously unused class, then the styling just doesn't apply at all.

For example, changing the color of the last div between bg-indigo-500 and bg-indigo-700, classes already used on the other two divs, results in the color updated correctly on save. But if I change it to a different one, for example bg-indigo-200, the background color just disappears. Hard refreshing the page, seems to get the new background color to turn up, but there have been a few times where it didn't, and I had to actually restart the dev-server...

Guessing a link between Tailwind/PostCSS and Next is broken or something? 🤷‍♂️

@spencer5051
Copy link

Same issue here. You can use the solution for Turbo Pack (concurrently) in the mean time.

npm install --save-dev concurrently

{
"scripts": {
"dev": "concurrently "next dev --turbo" "tailwindcss --input input.css --output output.css --watch"",
"build": "tailwindcss input.css --output output.css && next build"
}
}

https://beta.nextjs.org/docs/styling/tailwind-css

@Svish
Copy link

Svish commented Nov 27, 2022

For now I have just downgraded to 13.0.4, where it at least seems to work. Not sure what I'm missing from Next-wise from the versions above though... 🤔

@MattL-NZ
Copy link

Can confirm I am experiencing the same issues with 13.0.5 and tailwind as reported by @Svish. Going back to 13.0.4 is fine.

@thesampaton
Copy link

Just did some preliminary investigation, the defect looks to have been introduced in 13.0.5-canary.4

The canary versions prior to that do not have the issue, a few updates to the hot reloader types in that release

@volnei
Copy link

volnei commented Dec 1, 2022

For me it's not working even in previous versions :(

@rijk
Copy link

rijk commented Dec 1, 2022

Same, for me it has never worked on v13. Tailwind works but JIT classes are not added until you refresh the page.

@septem1997
Copy link

same problem with 13.0.6

@volnei
Copy link

volnei commented Dec 2, 2022

It become extremelly fast when move the @tailwind declarations to another file and import it in globals.css. But refresh somethings still fail. I've noticed that it works perfectly on client components, but not refreshing for server components.

@urwrstkn8mare
Copy link

urwrstkn8mare commented Dec 5, 2022

same problem with 13.0.6

Same fixed by downgrading to 13.0.4. For anyone wondering like me how to downgrade a npm package, you just install the package again with @<version> and it will overwrite. So: npm install next@13.0.4

@iamjohnlong
Copy link

iamjohnlong commented Dec 8, 2022

Tailwind and HRM is working on v13.0.4-canary.5 for me but I can't get it working on anything above it.

@ciruz
Copy link
Contributor

ciruz commented Dec 9, 2022

Tailwind and HRM is working on v13.0.4-canary.5 for me but I can't get it working on anything above it.

Just tested different next.js versions with tailwind because of the hot reload error and for me it works with 13.0.4-canary.5 (pre release) and 13.0.4 (release), but is broken with 13.0.5-canary.0.

@jimbits
Copy link

jimbits commented Dec 11, 2022

Downgrading to 13.0.4-canary.5 worked for me. MACOS Big Sur 11.4 Intel.
"dependencies": {
"@types/node": "18.11.13",
"@types/react": "18.0.26",
"@types/react-dom": "18.0.9",
"eslint": "8.29.0",
"eslint-config-next": "13.0.6",
"framer-motion": "^7.6.19",
"next": "^13.0.4-canary.5",
"react": "18.2.0",
"react-dom": "18.2.0",
"typescript": "4.9.4"
},
"devDependencies": {
"autoprefixer": "^10.4.13",
"postcss": "^8.4.19",
"tailwindcss": "^3.2.4",
"concurrently": "^7.6.0"
}

@rijk
Copy link

rijk commented Dec 12, 2022

Just did some preliminary investigation, the defect looks to have been introduced in 13.0.5-canary.4

The canary versions prior to that do not have the issue, a few updates to the hot reloader types in that release

Okay, just did some testing and can confirm this. I don't know how you tested, I just added a <strong className="text-[#22bdec]">test</strong> element with a different hex code each time. From 13.0.5-canary.4 the text is black after the hot reload, instead of the new color.

@rijk
Copy link

rijk commented Dec 12, 2022

Digging into the changes, I think this is the culprit:

image

Because I see the 'cache buster' suffix is no longer added to the stylesheet, making the browser unaware that its contents changed. If I refresh the stylesheet manually in a second tab, I do see the new class added to it.

rijk referenced this issue Dec 12, 2022
As discussed [here](https://vercel.slack.com/archives/C035J346QQL/p1668425692084449), we don't need the timestamp for CSS resources in the app dir for now as we are not preloading them. Manually tested with Safari and added corresponding e2e tests.

Closes #42862.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
@huozhi
Copy link
Member

huozhi commented Dec 15, 2022

We land a fix in 13.0.8-canary.0. Could you upgrade to to see if the hmr is fixed for you?

@typeofweb
Copy link
Contributor

@huozhi HMR is indeed working in 13.0.8-canary.0 when using Tailwind.
There's a small issue, though: a flash of unstyled content before new styles are applied.

@MattL-NZ
Copy link

@huozhi Working for me but I am also experiencing UI flashes similar to what @mmiszy mentioned.

@huozhi huozhi added kind: bug area: app App directory (appDir: true) and removed bug Issue was opened via the bug report template. labels Dec 15, 2022
@thesampaton
Copy link

thesampaton commented Dec 16, 2022

It seems to be working, though I hacked around some stuff to minimise the impact of this bug so need to validate imported stuff again. Will update if I it rears up during that refactoring but assume closed

@Svish
Copy link

Svish commented Jan 4, 2023

Tried to upgrade to version 13.1.1 now, but it's still broken, so I'm back at 13.0.4. 😕

Saw the Next.js 13.1 Explained! video, where he showed off tailwind with --turbo, where it seemed to work, so hoped it would be fixed now, but... nope... ☹

@zhangwei900808
Copy link

antd same problem

@malviyaritesh
Copy link

I was also facing this issue with tailwindcss. I'm using Next 13.2.4 and Safari 16.4.

After some research, I managed to workaround this issue by adding following in next.config.js:

headers:
    process.env.NODE_ENV === 'development'
      ? () => [
          {
            source: '/_next/static/css/_app-client_src_app_globals_css.css',
            headers: [{ key: 'Vary', value: '*' }],
          },
        ]
      : undefined,

Vary: *
Indicates that factors other than request headers influenced the generation of this response. Implies that the response is uncacheable.

This is working perfectly for me. You can read more about Vary: * on MDN.

@ncinme
Copy link

ncinme commented Apr 6, 2023

This is awesome @malviyaritesh. I confirm that with your solution, it is working for me with the latest Next.js v13.2.4 and tailwindcss 3.3.1 on both Chrome and Edge browsers.

@stathis1998
Copy link

stathis1998 commented Apr 6, 2023 via email

@timneutkens
Copy link
Member

Hey, you don't have to comment saying an issue still exists in "version x" because the issue is still open it is not fixed. Thanks!

@darklight9811
Copy link

@timneutkens I think it's valid, so the issue doesn't get tagged as stale and ends up thrown in stale oblivion (forgotten).

@timneutkens
Copy link
Member

CleanShot 2023-04-10 at 09 13 16@2x

When these labels are added it'll never be set as stale as it's triaged, we're aware of it, and no further action is needed as it's already being tracked.

@darklight9811
Copy link

@malviyaritesh your fix stopped working between the latest releases

@timneutkens
Copy link
Member

Adding the vary header has no effect on this issue, it needs special handling in React which is something we're looking into.

@stephtr
Copy link

stephtr commented Apr 16, 2023

I also noticed that importing the global.css file from a server component (as also done in the starter template) didn't enable fast-refresh with --turbo. Once I moved the import into a client component, fast-refresh is working, without the header setting. Using Edge @13.2.4

@akomm
Copy link

akomm commented Apr 17, 2023

@malviyaritesh

I'm not sure if the workaround really works. On my end it does not help. I see people say it works, but can they verify the following: check the css file generated by TW and see if the utility you test against fast-refresh isn't in the file already. TW in development does not truncate the utilities, so when you used them once for testing of fast-refresh and then changed back, the utility still remains in the css file and when you test it later again, it appears to work, but in fact it doesn't. You need to either remove the TW output file on each test or use always a new utility class that you've never used before.

TLDR;

FF never receives an even that would require fetching the file again. So it will not consult the cache. Because the link's href does not change, it needs some other way to cause the browser reload the file. The method used seem not to work in FF.

huozhi pushed a commit that referenced this issue Apr 19, 2023
…#48578)

Closes NEXT-684, closes #43396.

This PR implements a temporary workaround to address the issue that some
browsers are always caching CSS resources during the lifetime of a
session. We re-introduce the versioning query to the resource to avoid
that, and then use Mutation Observer to do GC manually on the client.
Once Float handles that by itself, we can probably remove this.

Note that correctly handling GC here is **required** for correctness,
not an optimization. That's why it took us a while to address this (even
this PR is still a temporary workaround). Imagine that if you have:

```css
h1 {
  color: red;
}
```

and then you changed it to:

```css
h1 {
  font-size: 300px;
}
```

During HMR, if we don't remove the old resources but only insert the new
one, both will be applied and you will still see the `<h1>` in red,
which is wrong.

Here's a recording of this PR working correctly in Firefox:


https://user-images.githubusercontent.com/3676859/233132831-b88e4d8d-aec9-48c4-9aa7-7c7a149b377d.mp4
@valentincostam
Copy link
Contributor

I updated Next.js to 13.3.1 (which includes #48578) and fast-refresh doesn't work. When I change a Tailwind class, the browser notices the changes but the styles are not applied. I have to refresh the page manually.

Did I miss anything? Is anyone experiencing the same?

Tried on:

  • Windows 11 Pro 22H2 Build 22621.1555
  • WSL2: Ubuntu 20.04
  • Firefox 112.0.1
  • Brave 1.50.121 (Chromium: 112.0.5615.138)

@akomm
Copy link

akomm commented May 2, 2023

Whoever still copes with the issue, here's the fix:

The issue is still present, but you can circumvent the issue by integrating tailwind differently. I'm not sure if this method was available before, but at least since next 13.3.1 you can integrate it via postcss as a plugin - so check your next version if it does not work. Then instead of importing the TW output css file, you import the TW input css file. The webpack loader chain this way recompiles the input file and reloads the link properly. Not need to run tailwind parallel anymore as its part of the webpack pipeline.

It has been added to the docs: https://beta.nextjs.org/docs/styling/tailwind-css

@timneutkens timneutkens reopened this May 4, 2023
@eduardochiaro
Copy link

@akomm followed what you said it works, but I notice that the issue is still present if you use scss instead if css as global file. Issue with SASS i guess, but it seems to work fine if applied to a pages page instead.

@akomm
Copy link

akomm commented May 8, 2023

@eduardochiaro I did not test it with scss, but did you integrate scss as postcss plugin? Can you also verify that the postcss plugin is used and not just scss->css->style loaders that would normally by used when you globally import scss in next?

@eduardochiaro
Copy link

eduardochiaro commented May 8, 2023

@eduardochiaro I did not test it with scss, but did you integrate scss as postcss plugin? Can you also verify that the postcss plugin is used and not just scss->css->style loaders that would normally by used when you globally import scss in next?

@akomm I don't think is really needed, since nextjs should support it. https://nextjs.org/docs/app/building-your-application/styling/sass
I can tell you that if you use pages folder, you don't have this issue, all works fine and hot reload shows update for your css changes, app folder only do that if you update directly a page.tsx. If your style is in an imported component, it will need a browser refresh to show.
I will try adding directly the plug-in to see if make a difference.

@shuding
Copy link
Member

shuding commented May 8, 2023

@akomm @eduardochiaro @valentincostam is it possible for you to create a small repository to reproduce this problem?

@eduardochiaro
Copy link

eduardochiaro commented May 8, 2023

@shuding I created this https://github.com/eduardochiaro/test-sass
I set up 2 pages, one under app folder, the other under pages folder. Both using same scss global file and component.
To test, open homepage / and also /test page.

  • http://localhost:3000/ use app folder,
  • http://localhost:3000/test use pages folder.

Using next dev make a change on \src\app\Component.tsx. the change need to be a tailwind call not used before. Hot reload will trigger on http://localhost:3000/test but not on http://localhost:3000/.

shuding added a commit that referenced this issue May 9, 2023
…tions (#49462)

In Hot Reloader we use `mod.resource.replace(/\\/g, '/').includes(key)`
to see if the module is the entry. However that `includes` check isn't
solid. Say the entry key is `app/path/page`, these will all be matched:

```
app/path/page.js
app/path/page.css
app/unrelated/app/path/page/another/page.js
```

This PR fixes that and added a test case. I'm unsure if this fixes the
newly reported cases in #43396.

fix NEXT-1110
@kodiakhq kodiakhq bot closed this as completed in #49534 May 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.