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

fix(packages/typescript): Fix not to overwrite tsconfig #5356

Closed
wants to merge 2 commits into from
Closed

fix(packages/typescript): Fix not to overwrite tsconfig #5356

wants to merge 2 commits into from

Conversation

sawa-zen
Copy link

@sawa-zen sawa-zen commented Mar 23, 2019

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

The packages/typescript setup process is breaking the existing tsconfig.
For example, there is the following destruction.

{
  ...
  "compilerOptions": {
    "lib": [
      "dom",
      "es2015"
    ]
  },
  ...
}
↓
{
  "compilerOptions": {
    "lib": [
      "dom",
      "es2015",
      "dom"
    ]
  },
}

This is a problem due to using loash.defaultsDeep.
lodash/lodash#2801

But, I thought that overriding the default tsconfig settings to the existing tsconfig was a problem. So I changed to create a default tsconfig only when tsconfig did not exist.

Fixes: #5338

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.

@codecov-io
Copy link

codecov-io commented Mar 23, 2019

Codecov Report

Merging #5356 into dev will decrease coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##            dev   #5356      +/-   ##
=======================================
- Coverage    96%     96%   -0.01%     
=======================================
  Files        74      74              
  Lines      2529    2528       -1     
  Branches    640     641       +1     
=======================================
- Hits       2428    2427       -1     
  Misses       85      85              
  Partials     16      16
Impacted Files Coverage Δ
packages/cli/src/utils/typescript.js 27.27% <0%> (-3.5%) ⬇️
packages/typescript/src/index.js 88.88% <66.66%> (-11.12%) ⬇️

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 77dcfe6...45359db. Read the comment docs.

@kevinmarrec
Copy link
Contributor

@sawa-zen Thanks for your time, we already planned to change this behavior, we will switch back to the extends way. So there won't be defaultDeeps anymore.

@danielroe
Copy link
Member

This should also fix deploying Nuxt 2.5.x with TypeScript on Now, which currently produces the following fatal error:

FATAL  EACCES: permission denied, open '/home/nowuser/src/tsconfig.json'

@galvez galvez requested a review from kevinmarrec March 26, 2019 08:15
@pi0 pi0 closed this in #5367 Mar 29, 2019
pi0 pushed a commit that referenced this pull request Mar 29, 2019
Co-authored-by: SAWADA Takayoshi <sawadasuiren@gmail.com>
@pi0
Copy link
Member

pi0 commented Mar 29, 2019

Merged in #5367

@sawa-zen sawa-zen deleted the fix/create-tsconfig branch March 29, 2019 18:39
@pi0 pi0 mentioned this pull request Mar 31, 2019
@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants