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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add styleExtensions #4671

Merged
merged 3 commits into from Jan 2, 2019

Conversation

Projects
None yet
4 participants
@manniL
Copy link
Member

manniL commented Jan 2, 2019

In your css array, you have to write the corresponding extension when providing your stylesheets. While transitioning from SCSS to PostCSS in several projects, I thought: "Why not completely omitting the extension as we do it for JS resolves 馃?"

There we go!

Types of changes

  • New feature (a non-breaking change which adds functionality)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.
@@ -46,7 +46,7 @@ export default class Resolver {
return resolve(this.options.srcDir, path)
}

resolvePath(path, { alias, module } = {}) {
resolvePath(path, { alias, module, isStyle } = {}) {

This comment has been minimized.

@manniL

manniL Jan 2, 2019

Member

Maybe we should move to isXYZ syntax generally for booleans here? 馃

This comment has been minimized.

@pi0

pi0 Jan 2, 2019

Member

I like this convention 馃憤

@manniL manniL requested review from pi0 and clarkdo Jan 2, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 2, 2019

Codecov Report

Merging #4671 into dev will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #4671      +/-   ##
==========================================
+ Coverage   90.78%   90.82%   +0.04%     
==========================================
  Files          67       67              
  Lines        2236     2224      -12     
  Branches      545      544       -1     
==========================================
- Hits         2030     2020      -10     
+ Misses        187      185       -2     
  Partials       19       19
Impacted Files Coverage 螖
packages/config/src/config/_common.js 100% <酶> (酶) 猬嗭笍
packages/core/src/resolver.js 86.79% <100%> (+0.25%) 猬嗭笍
packages/config/src/options.js 94.11% <0%> (-2.1%) 猬囷笍
packages/vue-app/src/index.js 0% <0%> (酶) 猬嗭笍
packages/builder/src/builder.js 96.7% <0%> (+3.17%) 猬嗭笍

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 7dd33fe...2fb50c1. Read the comment docs.

@galvez
Copy link
Member

galvez left a comment

LGTM!

@pi0
Copy link
Member

pi0 left a comment

Nice enhancement. But I don't see any reason adding complexity for handling option as a function or string unless a specific use case exists. replace or append merge strategy would be enough and more consistent. Modules has the opprotounity to modify options in any manner they need :)

@@ -46,7 +46,7 @@ export default class Resolver {
return resolve(this.options.srcDir, path)
}

resolvePath(path, { alias, module } = {}) {
resolvePath(path, { alias, module, isStyle } = {}) {

This comment has been minimized.

@pi0

pi0 Jan 2, 2019

Member

I like this convention 馃憤

Show resolved Hide resolved packages/config/src/options.js Outdated
Show resolved Hide resolved packages/config/src/options.js Outdated
@@ -43,6 +43,21 @@ export function getNuxtConfig(_options) {
options.extensions = [options.extensions]
}

const styleExtensionDefaults = ['css', 'pcss', 'postcss', 'styl', 'stylus', 'scss', 'sass', 'less']

This comment has been minimized.

@pi0

pi0 Jan 2, 2019

Member

If defaults ordering would be important I suggest mentioning it in the docs too :)

This comment has been minimized.

@manniL

manniL Jan 2, 2019

Member

Yeah, I'll create a doc PR afterwards anyway 鈽猴笍
Thanks for the heads up!

@manniL manniL dismissed pi0鈥檚 stale review via 2fb50c1 Jan 2, 2019

@pi0

pi0 approved these changes Jan 2, 2019

@pi0 pi0 merged commit 471a32a into dev Jan 2, 2019

9 checks passed

Semantic Pull Request ready to be squashed
Details
[ci.azure] nuxt.js #20190102.21 succeeded
Details
ci/circleci: audit Your tests passed on CircleCI!
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: test-e2e Your tests passed on CircleCI!
Details
ci/circleci: test-unit Your tests passed on CircleCI!
Details
security/snyk - package.json (Atinux) No new issues
Details

@pi0 pi0 deleted the feat-resolve-style-paths-to-omit-endings branch Jan 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment