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

feat(config): support .nuxtrc #7430

Merged
merged 4 commits into from
May 28, 2020
Merged

feat(config): support .nuxtrc #7430

merged 4 commits into from
May 28, 2020

Conversation

pi0
Copy link
Member

@pi0 pi0 commented May 28, 2020

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Support .nuxtrc (per project and user home) to extend nuxt.config without making modifications. The use case is for any module that wants to persist a state or config per project on an automated basis like telemetry module to set telemetry.enabled. RC format is flattened INI but config is read unflattened with native types (see rc9)

Loading priority: configOverrides > nuxtConfig > .nuxtrc > .nuxtrc (global)

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.

@pi0 pi0 requested review from clarkdo, Atinux and a team May 28, 2020 14:48
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2020

Codecov Report

Merging #7430 into dev will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #7430   +/-   ##
=======================================
  Coverage   70.26%   70.27%           
=======================================
  Files          88       88           
  Lines        3696     3697    +1     
  Branches     1008     1008           
=======================================
+ Hits         2597     2598    +1     
  Misses        892      892           
  Partials      207      207           
Flag Coverage Δ
#unittests 70.27% <100.00%> (+<0.01%) ⬆️
Impacted Files Coverage Δ
packages/config/src/load.js 77.10% <100.00%> (+0.27%) ⬆️

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 2e711c1...4029901. Read the comment docs.

Copy link
Member

@clarkdo clarkdo left a comment

Choose a reason for hiding this comment

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

The change looks good.

I'm wondering if it's doing different things like what most framworks are doing, like babel won't have babel.config.js --falback-to--> .babelrc, eslint wont have .eslintrc.js --falback-to--> .eslintrc.

Another point is : Regarding to use rc file, is there anything impossible or hard to be implemented by using nuxt.config.js (or maybe a global nuxt.config.js) ?

@pi0
Copy link
Member Author

pi0 commented May 28, 2020

I'm wondering if it's doing different things like what most framworks are doing, like babel won't have babel.config.js --falback-to--> .babelrc, eslint wont have .eslintrc.js --falback-to--> .eslintrc.

Indeed this is same behavior we will have. (both loaded) The current loadConfig fixture for nuxtrc does not have a nuxt.config.js file. Main different with other (js) frameworks, is that nuxtrc is flat INI that allows easy modifications by automated tools (even a bash script) but loaded as valid json.

Another point is : Regarding to use rc file, is there anything impossible or hard to be implemented by using nuxt.config.js (or maybe a global nuxt.config.js) ?

The most advantage is updating as for nuxt.config we need to parse syntax with different possibilities ({typescript, esm, cjs} x {exported object, exported function}) compared to a dead-simple syntax and using rc.update(). It also makes nuxt.config more readable by hiding tokens or configs like telemetry into a plain file. Another advantage is that this file can be optionally checked-in into a repository so based on project requirements can be gitignored or not.

@clarkdo
Copy link
Member

clarkdo commented May 28, 2020

So if we always respect nuxt.config.js, .nuxtrc and ~/.nuxtrc at same time, do we need to provide a way to at least disable ~/.nuxtrc in an app for edge cases ?

@pi0
Copy link
Member Author

pi0 commented May 28, 2020

So if we always respect nuxt.config.js, .nuxtrc and ~/.nuxtrc at same time, do we need to provide a way to at least disable ~/.nuxtrc in an app for edge cases ?

Agree. If we find a valid problematic case can support CLI flag but I don't think it would be necessary for initial support. PS: nuxt.config always has most priority and overrides nuxtrc.

Atinux
Atinux previously approved these changes May 28, 2020
packages/config/src/load.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants