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(misc): do not generate tsconfig.base.json for simple standalone … #13605

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

vsavkin
Copy link
Member

@vsavkin vsavkin commented Dec 2, 2022

…repos

Current Behavior

Expected Behavior

Related Issue(s)

Fixes #

@vercel
Copy link

vercel bot commented Dec 2, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
nx-dev ✅ Ready (Inspect) Visit Preview Dec 4, 2022 at 6:36PM (UTC)

@vsavkin vsavkin force-pushed the remove-tsconfig-base branch from 72417ef to c3ffabe Compare December 2, 2022 21:55
@nx-cloud
Copy link

nx-cloud bot commented Dec 2, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 038ee08. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 20 targets

Sent with 💌 from NxCloud.

packages/angular/src/generators/utils/create-ts-config.ts Outdated Show resolved Hide resolved
packages/angular/src/generators/utils/create-ts-config.ts Outdated Show resolved Hide resolved
for (let compilerOption of Object.keys(tsConfigBaseOptions)) {
baseCompilerOptions[compilerOption] =
tsconfig.compilerOptions[compilerOption];
delete tsconfig.compilerOptions[compilerOption];
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're going to end up with an empty compilerOptions? Why not just copy the whole object over and delete the original?

Copy link
Member Author

Choose a reason for hiding this comment

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

Compiler options can change. People can add new options etc. Some compiler options are framework specific

@@ -165,15 +155,6 @@ describe('app', () => {
const tsConfig = readJson(appTree, 'apps/my-app/tsconfig.json');
expect(tsConfig.extends).toEqual('../../tsconfig.base.json');
});

it('should extend from root tsconfig.json when no tsconfig.base.json', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

It no longer works like that. It will just extract tsconfig.base when ti is missing. So I updated one of the tests.

Bur for some reason we retest everything twice (with nested dirs and without), even though most of what we test has nothing to do with nested dirs.

packages/react/src/utils/create-ts-config.ts Outdated Show resolved Hide resolved
packages/react/src/utils/create-ts-config.ts Outdated Show resolved Hide resolved
@FrozenPandaz FrozenPandaz merged commit 91c19f5 into nrwl:master Dec 5, 2022
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
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.

2 participants