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

Adding jsx and sass extensions to webpack loader config #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding jsx and sass extensions to webpack loader config #2

wants to merge 1 commit into from

Conversation

ab18556
Copy link

@ab18556 ab18556 commented May 11, 2017

I've notice that the sass loader's regex was not matching.sass extension, and the js loader was matching .j instead of .jsx

I'm new to webpack tho so I might be wrong or not understanding some concepts

@alanbsmith
Copy link
Owner

alanbsmith commented May 11, 2017

Hey @ab18556 !

Thanks you so much for this! Most React components use a .js extension (in my experience). But the loader should catch .js or .jsx? I might be wrong on that. The sass loader update is spot on. I typically use .scss extensions, but the loader should allow both scss and sass extensions.

@ab18556
Copy link
Author

ab18556 commented May 11, 2017

Hi @alanbsmith ,

About .jsx, I was asking myself which file extension should I personnaly use. I decided to go with the community and use .js, but much people still prefer .jsx

I think the .j extension should not be matched though.

About .sass, I prefer .scss too, but why not including both and let people choose? :)

Nice little project btw, I was looking for a concise setup that would help me to do electron+react apps so this is exactly what I was looking for.

Thanks,
Pat

@alanbsmith
Copy link
Owner

Yeah, I'm happy to support both .js and .jsx as well as scss and sass. I thought the line that's there would catch both.

{ test: /\.js?$/, loader: 'babel', exclude: /node_modules/ },

But I could be wrong. I haven't looked at this starter in a while. I can verify that sometime this morning though. Thanks again for the PR! We'll get it merged soon!

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.

2 participants