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

Add example with-scoped-stylesheets-and-postcss #1146

Merged
merged 6 commits into from
Feb 17, 2017
Merged

Add example with-scoped-stylesheets-and-postcss #1146

merged 6 commits into from
Feb 17, 2017

Conversation

thomaslindstrom
Copy link
Contributor

🤡

@davibe
Copy link
Contributor

davibe commented Feb 15, 2017

Unless I am misunderstanding something this is basically the same as my example with-global-stylesheet. Even if slighty different the technique is the same so maybe it would make better sense to merge the efforts making one single good example OR moving loaders inside next.js itself. Examples are already very crowded.

@thomaslindstrom
Copy link
Contributor Author

thomaslindstrom commented Feb 15, 2017

@davibe Hey, sorry, yes. I copied your with-global-stylesheet, but forgot to attribute you in the README.md. I don't know about merging the examples – I personally found them to lack in the fact that none really solved my problem, so I made this one: More specific examples are better than a few general ones, IMO.

I think your example is useful for someone who don't want postcss with css-modules, but wants to use sass instead.

I'll definitely add an attribution, though.

@thomaslindstrom
Copy link
Contributor Author

I don't know why the tests keep failing... They all pass locally, for me, and this PR shouldn't affect anything.

@timneutkens
Copy link
Member

@thomaslindstrom from the looks of it, it has nothing to do with your PR.

@thomaslindstrom
Copy link
Contributor Author

@timneutkens Yeah, no, it finally passed now!

@davibe
Copy link
Contributor

davibe commented Feb 15, 2017

Thank you, I was not looking for attribution. I am just saying its redundant to have 2 examples of the same technique. I picked a generic name "*-global-stylesheet" to intentionally accomodate multiple engines. Anyway, its just my point of view.

@thomaslindstrom
Copy link
Contributor Author

thomaslindstrom commented Feb 15, 2017

It was out of courtesy.

I see where you're coming from, but I disagree. “I don't have the time to set up my own build configurations and figure out how everything works.” This is the beauty of next.js – everything should just work out of the box. And, when it doesn't, I think there should be examples that fit your use case almost perfectly.

@rauchg
Copy link
Member

rauchg commented Feb 15, 2017

I personally like this example a lot. It's ok to err on more examples rather than fewer, provided they have good explanations, they work and we maintain them.

I would love to get rid of the "known bug" though :\

@thomaslindstrom
Copy link
Contributor Author

thomaslindstrom commented Feb 15, 2017

@rauchg Yeah, me too... I am not sure where it comes from, but I assumed it had something to do with #544 and #627 (comment), and the fact that webpack, AFAIK, is ran from within next inside node_modules (some path stuff going on here?).

Module build failed: Error: Cannot find module '-!./../node_modules/css-loader/index.js??ref--6-4!./../node_modules/postcss-loader/index.js!../global.css'

I'm not even sure what module it has trouble finding. It could be that it doesn't find ../global.css. Could this be fixed by adding some config into webpack that sets some kind of directory context?

@davibe
Copy link
Contributor

davibe commented Feb 16, 2017

Can we have consistent naming so that the two examples are shown close together ? I refer to with-(global|external)-stylesheet*. I vote for global because it is better opposes to scoped

@thomaslindstrom
Copy link
Contributor Author

Could probably rename this to with-scoped-stylesheets-and-postcss?

@thomaslindstrom thomaslindstrom changed the title Add example with-external-stylesheets-and-postcss Add example with-scoped-stylesheets-and-postcss Feb 16, 2017
@thomaslindstrom
Copy link
Contributor Author

I changed the name of the example to with-scoped-stylesheets-and-postcss. It still fails due to some random error, though.

@timneutkens timneutkens merged commit 6e5d57d into vercel:master Feb 17, 2017
@timneutkens
Copy link
Member

It's not from this PR @thomaslindstrom, so that's fine. I merged it in, could you create an issue for the known bug with steps to reproduce? ❤️

@thomaslindstrom
Copy link
Contributor Author

#1191 😘

@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
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 this pull request may close these issues.

4 participants