allow tsconfig to contain emitDeclarationOnly=true

[entropitor]

Fixes #10796
Fixes #9373
Fixes #8551

Typescript project references are a useful feature in a monorepo that allow building sub-packages incrementally and don't require a rebuild of a dependency to get up-to date type information in the editor of that dependency.

However, for it to work, some fields need to be set in the tsconfig: composite: true, incremental: true, emitDeclarationOnly: true the last one is a problem because typescript errors when there is a field for noEmit and for emitDeclarationOnly. This PR inspect the users' config, and if they already have a emitDeclarationOnly, it needs to be set to the value true and if they don't the current behaviour is kept.

I've tried copying this file to my local project's node_modules and now I can add emitDeclarationOnly without a problem (and thus without react-scripts overwriting my tsconfig)

[facebook-github-bot]

Hi @entropitor!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[entropitor]

@ianschmitz @iansu @mrmckeb @petetnt Could you please review?

[stale]

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[entropitor]

Who can review this PR?

[mrmckeb]

Hi @entropitor, are you able to use a tsconfig.build.json or similar in this scenario?

[entropitor]

@mrmckeb What do you mean? This script is working just fine as is, no need for a tsconfig.build.json. create-react-app is enforcing too strict limits on what the tsconfig.json can contain, this PR fixes that

[VitorLuizC]

Hi @entropitor, are you able to use a tsconfig.build.json or similar in this scenario?

No, I tried it too but CRA updates tsconfig.json to include "noEmit": true,. Seems like its a required option, and the problem its that with it we can't generate our package's type declarations...

A workaround is using pre/post scripts to manipulate tsconfig, but it isn't a good pratice and causes issues when we run more than one script (start, test)

[entropitor]

@ianschmitz @iansu @mrmckeb @petetnt What do I need to do to get this simple change in?

[mikehardy]

I reviewed this carefully including reading related issues and documentation.
I tested it carefully locally with a monorepo that requires types from my react-app to be used in a set of firebase functions in a different module, along with a react-native-web overlay (so metro bundler consumption of tsconfig + ts[x] files was also checked)

A tsconfig with emitDeclarationsOnly=true, noEmit=false, incremental=true works great.

Thank you so much for investigating this and fixing it @entropitor

This should be merged, it appears most recent previous committers were @MadJose @VitorLuizC and @KonstantinSimeonov perhaps they can lend a hand.

I'm attaching a diff in patch-package format - just remove the .txt extension I had to add so github would allow me to attach it - this allows me to apply this locally so I'm unblocked. Perhaps others may find it useful, patch-package is perfect for integrating changes locally while waiting for PR processes.

react-scripts+4.0.3.patch.txt

[stale]

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[mikehardy]

This is not stale. This PR functions correctly, seems to go along with the philosophy of the repo and is in use in production apps

Please merge 🙏

[mikehardy]

@entropitor not sure if it would help, but you could perhaps rebase this against current master and push it back out, that might re-trigger the CI runs and perhaps they would build? At the moment the failing tests have been removed so it is not even possible to see if the failed builds were because of this or some other flakiness

[entropitor]

@mikehardy done

[mikehardy]

Nice! All green on the CI runs that don't need approval. I'm not a maintainer here, but hopefully that makes the PR look a little better for their limited attention. Cheers

[smac89]

LGTM! 🚢

[smac89]

While the maintainers are away on vacation, anyone who really wants to get around this issue can try the following script (no-tsconfig-mod.ts), which makes use of the pirates module to intercept the import for verifyTypescriptSetup.js in react-scripts:

import { addHook } from "pirates";

const matcher = (filename: string) => filename.endsWith("react-scripts/scripts/utils/verifyTypeScriptSetup.js");

addHook(() => "module.exports = () => {}", { ignoreNodeModules: false, matcher });

If you use craco, you can do import "./no-tsconfig-mod"; at the root of your craco.config.(ts|js) file, and it should immediately start working.