Skip to content
This repository has been archived by the owner on Jul 6, 2021. It is now read-only.

Unable to use inline CSS after switching to mini-css-extract-plugin #238

Closed
kachkaev opened this issue Aug 3, 2018 · 20 comments
Closed

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Aug 3, 2018

Since recently (#228), it is no longer necessary to add <link rel="stylesheet" href="/_next/static/style.css" /> to _document.js because <link rel="stylesheet" href="/_next/static/css/styles.HASH.chunk.css"/> is appended automatically. This is easy for developers, but comes at a cost for users. In my case, it blocks rendering by ≈1 second and shows the following warning in the lighthouse report:

External stylesheets are blocking the first paint of your page. Consider delivering critical CSS via
<style> tags and deferring non-critical styles.

See original discovery of this performance issue in #11 (comment).

It'd be great to be able to use <style>...inline...</style> instead of <link rel="stylesheet" /> (perhaps, as an opt-in). This approach would be especially useful in a project with styled-components, where pretty much anything except normalize.css and a couple of global CSS rules is added to the body of a page by that library. How could we use inline styles in _document.js?

@dotlouis
Copy link

dotlouis commented Aug 31, 2018

Same problem here.
Having a way to import styles and inline them would be very helpful to improve perf.
Any update on this?

I saw that styled-jsx v3 allows for importing css files and use them inine but this version is not shipped with next@6.
https://github.com/zeit/styled-jsx#styles-in-regular-css-files

@arianon

This comment has been minimized.

@kachkaev

This comment has been minimized.

@badpenguin
Copy link

Does not work for me, it tries to copy css from ./_next/ but instead the css is inside ./out/_next/
Is there a way to add the correct path prefix?

@virzak
Copy link

virzak commented Mar 11, 2019

This is a great solution!
The only issue is when scss gets modified, there is no automatic refresh. The only way is F5. Any idea how to resolve that?

@badpenguin
Copy link

It works for me in v7 https://github.com/badpenguin/nextjs-static-generator-boilerplate
But in v8.1 there is some weird things happening with this.context._documentProps.files vercel/next.js#7641

@HimanJusta

This comment has been minimized.

@arianon
Copy link

arianon commented Nov 18, 2019

@HimanJusta I tried to replicate your error (I don't use Zeit Now) but it works fine. https://himanjusta-issue-238-comment.arianon.now.sh/

Thought that file path in the error does seem weird to me, shouldn't the path be /var/task/.next/static/css/commons.b2e899ec.chunk.css? note the . in .next

@HimanJusta
Copy link

HimanJusta commented Nov 18, 2019

@HimanJusta I tried to replicate your error (I don't use Zeit Now) but it works fine. https://himanjusta-issue-238-comment.arianon.now.sh/

Thought that file path in the error does seem weird to me, shouldn't the path be /var/task/.next/static/css/commons.b2e899ec.chunk.css? note the . in .next

Im facing this issue when i deploy to Zeit now, if I just use npm start and see the prod build on localhost it works. Regarding the error in file path I did notice that, but looks like zeit now uploads the next build to _next below is the snapshot of the final folder stucture in now
Screenshot 2019-11-18 at 7 19 54 PM

Screenshot 2019-11-18 at 7 21 05 PM

Not sure if I need to add anything in now.json or not

@michalisgar
Copy link

@HimanJusta same issue for me, too. Have you had any luck?

@yanv1991
Copy link

yanv1991 commented Jun 4, 2020

that won't work if your server load your css files from an external source like s3

@kachkaev
Copy link
Contributor Author

kachkaev commented Jun 4, 2020

A got a notification from github and was surprised to discover that this was my issue 😅

Nearly two years since creating it, I can say that I’ve solved this problem for myself by using styled-normalize. Example:

Thus, in my recent apps, all styles come from styled-componets and there is not a single css file in the whole folder tree.

That said, I think it's worth keeping the issue open because it is still not possible to inline 'classic' global css in Next. Just sharing this update to help folks who can use the same trick as me 🙌

FYI: css is now a bult-in feature, no plugin is necessary: https://nextjs.org/docs/basic-features/built-in-css-support

@KATT
Copy link

KATT commented Jun 16, 2020

@kachkaev yes, it's built-in, but it doesn't inline it on the page and I ended up here from googling on how to do it :)

I'm using a

import '../styles/all.scss';

I simply have an old fashioned global SCSS-file - Ideally I would just embed it in a <style>-block on the page - does anyone know if this is possible?

Edit: posted question here

@rstacruz
Copy link

The solution in #238 (comment) still works well today (Next.js 9.5, July 2020).

It gives a warning, though I don't think it'd cause any impact:

Warning: next-head-count is missing. https://err.sh/next.js/next-head-count-missing

@carloscuesta
Copy link

carloscuesta commented Aug 11, 2020

On v9.5.2 which released today August 11, 2020. The solution at #238 (comment) returns the following error:

Unhandled error during request: TypeError: Cannot destructure property 'assetPrefix' of 'this.context._documentProps' as it is undefined.

You should now get this two variables from this.context to make it work again ✅

- const { assetPrefix, files } = this.context._documentProps
+ const { assetPrefix, files } = this.context

@amineo
Copy link

amineo commented Sep 4, 2020

In v9.5.3, this.context does not contain the files key so the solution outlined in #238 (comment) doesn't generate embedded styles anymore. It appears the previous files list has moved into the buildManifest.pages key where each page has its own list of files.

This minor change did seem to fix it for me (YMMV). I'm still getting my feet wet with Next so maybe someone with a little more experience can chime in to see if this is an appropriate fix?

        __getInlineStyles() {
-               const { assetPrefix, files } = this.context;
+               const { assetPrefix, buildManifest } = this.context;
 
-               if (!files || files.length === 0) return null;
+               if (!buildManifest.pages || buildManifest.pages.length === 0) return null;
 
-               return files.filter((file) => /\.css$/.test(file)).map((file) => (
+               const pageKeys = [];
+               for (const key of Object.keys(buildManifest.pages)) {
+                       if (buildManifest.pages[key]) {
+                               pageKeys.push(buildManifest.pages[key]);
+                       }
+               }
+               const mergeDedupeFiles = (arr) => {
+                       return [ ...new Set([].concat(...arr)) ];
+               };
+
+               return mergeDedupeFiles(pageKeys).filter((file) => /\.css$/.test(file)).map((file) => (
                        <style
                                key={file}
                                nonce={this.props.nonce}

My updated __getInlineStyles function

	__getInlineStyles() {
		const { assetPrefix, buildManifest } = this.context;

		if (!buildManifest.pages || buildManifest.pages.length === 0) return null;

		const pageKeys = [];
		for (const key of Object.keys(buildManifest.pages)) {
			if (buildManifest.pages[key]) {
				pageKeys.push(buildManifest.pages[key]);
			}
		}
		const mergeDedupeFiles = (arr) => {
			return [ ...new Set([].concat(...arr)) ];
		};

		return mergeDedupeFiles(pageKeys).filter((file) => /\.css$/.test(file)).map((file) => (
			<style
				key={file}
				nonce={this.props.nonce}
				data-href={`${assetPrefix}/_next/${file}`}
				dangerouslySetInnerHTML={{
					__html: fs.readFileSync(path.join(process.cwd(), '.next', file), 'utf-8')
				}}
			/>
		));
    }

Hope this helps someone!

@jsiddiqui
Copy link

jsiddiqui commented Sep 22, 2020

In 9.5.3, files are passed as argument to getCssLinks so no need to go through buildManifest:

getCssLinks({ allFiles }) {
  return allFiles
    .filter((file) => file.endsWith('.css'))
    .map((file) => (
      <style
        key={file}
        nonce={this.props.nonce}
        dangerouslySetInnerHTML={{
          __html: fs.readFileSync(path.join('.next', file), 'utf-8'),
        }}
      />
    ));
}

@deadcoder0904
Copy link

@jsiddiqui How do I do it in TypeScript?

When I click on Head in VSCode, I get the following:

getCssLinks(files: DocumentFiles): JSX.Element[] | null;

And DocumentFiles is defined as:

declare type DocumentFiles = {
    sharedFiles: readonly string[];
    pageFiles: readonly string[];
    allFiles: readonly string[];
};

Right now, I'm using any like:

getCssLinks({ allFiles }: { allFiles: any }) {
	const { assetPrefix } = this.context
	if (!allFiles || allFiles.length === 0) return null

	return allFiles
		.filter((file: any) => /\.css$/.test(file))
		.map((file: any) => (
			<style
				key={file}
				nonce={this.props.nonce}
				data-href={`${assetPrefix}/_next/${file}`}
				dangerouslySetInnerHTML={{
					__html: fs.readFileSync(path.join(process.cwd(), '.next', file), 'utf-8'),
				}}
			/>
		))
}

But why not export DocumentFiles so I can do the following instead:

getCssLinks({ allFiles }: DocumentFiles) {
  ...
}

@jsiddiqui
Copy link

@deadcoder0904 Specifying a type here is unnecessary as Typescript is able to infer the correct type due to extending the base class.

For example if you do this:

getCssLinks({ allFiles, test }) {

I get the following Typescript error:

src/pages/_document.tsx:9:3 - error TS2416: Property 'getCssLinks' in type 'InlineStylesHead' is not assignable to the same property in base type 'Head'.
  Type '({ allFiles, test }: { allFiles: any; test: any; }) => any' is not assignable to type '(files: DocumentFiles) => Element[]'.
    Types of parameters '__0' and 'files' are incompatible.
      Property 'test' is missing in type 'DocumentFiles' but required in type '{ allFiles: any; test: any; }'.

Therefore, I think that there is no need to export this.

@timneutkens
Copy link
Member

Hi, thanks for creating an issue. We currently recommend using https://nextjs.org/docs/basic-features/built-in-css-support as zeit/next-css and zeit/next-sass have been deprecated in favor of the built-in support.

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

No branches or pull requests