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

chore(gatsby): upgrade null-loader #22410

Merged
merged 1 commit into from Mar 20, 2020

Conversation

beliayeu
Copy link
Contributor

Description

see #22409

Related Issues

Fixes #22409

@beliayeu beliayeu requested a review from a team as a code owner March 19, 2020 17:45
@pieh
Copy link
Contributor

pieh commented Mar 19, 2020

There are few breaking changes on the way - https://github.com/webpack-contrib/null-loader/blob/master/CHANGELOG.md

Will have to wait for #22400 to be able to use v3 of null-loader (because right now we have 8.0 support still)

@beliayeu
Copy link
Contributor Author

@pieh, as an option null-loader can be upgraded to v2 since v2 contains the fix for the problem I encountered and per my understanding v2 is node 8 compatible. btw, is there a better pattern how to exclude assets from build? lets assume there is a css file imported by a component, we want to include this file while in gatsby develop but exclude it during gatsby build

@pieh
Copy link
Contributor

pieh commented Mar 19, 2020

@pieh, as an option null-loader can be upgraded to v2 since v2 contains the fix for the problem I encountered and per my understanding v2 is node 8 compatible.

We do plan to drop Node 8 tomorrow - we worked out most of CI problems today (but there can be delays), so if you are fine with holding to this PR a bit longer we can keep null-loader@3. The note was as much for you as much for anyone else checking this PR after me ;)

is there a better pattern how to exclude assets from build? lets assume there is a css file imported by a component, we want to include this file while in gatsby develop but exclude it during gatsby build

This is probably best way to do this, but I'm pretty confused about aplying null-loader just for build step. You ultimately do have to add styles somehow to your site?

@beliayeu
Copy link
Contributor Author

We do plan to drop Node 8 tomorrow - we worked out most of CI problems today (but there can be delays), so if you are fine with holding to this PR a bit longer we can keep null-loader@3

sounds great. I am totally fine

This is probably best way to do this, but I'm pretty confused about aplying null-loader just for build step. You ultimately do have to add styles somehow to your site?

the styles are added in gatsby-browser.js and in components. we have some styles that are needed only while we are in gatsby develop (styles needed for admin ui), that is why I was looking for a way how to exclude those admin styles from static build

@pieh
Copy link
Contributor

pieh commented Mar 19, 2020

the styles are added in gatsby-browser.js and in components. we have some styles that are needed only while we are in gatsby develop (styles needed for admin ui), that is why I was looking for a way how to exclude those admin styles from static build

not ideal, but you probably could try something like this (pretty sure import syntax won't work with conditional like that, that's why it's require):

if (process.env.BUILD_STAGE !== `build-javascript`) {
  require(`./layout.css`)
}

We use similar thing in gatsby ( https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/cache-dir/public-page-renderer.js )

@pieh
Copy link
Contributor

pieh commented Mar 19, 2020

Just note, that I don't think we document BUILD_STAGE "env var" (it's not really env var, it's using webpack's DefinePlugin), so this is not really part of public API

@pieh pieh changed the title fix(gatsby): upgraged null-loader so to be able to exclude imported css from builds chore(gatsby): upgrage null-loader Mar 20, 2020
Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! node 8 was just dropped so we can go ahead with this

@pieh pieh changed the title chore(gatsby): upgrage null-loader chore(gatsby): upgrade null-loader Mar 20, 2020
@pieh pieh merged commit 07b1434 into gatsbyjs:master Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to exclude an imported css file from static build
2 participants