Adds TSC_COMPILE_ON_ERROR env var... #6931

kylebebak · 2019-04-26T20:51:35Z

Closes #6930.

Allows users to run and properly build TypeScript projects even if there are TypeScript type check errors.

Motivation and discussion in this issue.

I know it works because I took the react-scripts and react-dev-utils deps from the create-react-app monorepo, edited them with the same changes that are in this PR, and put them in my own repos.

To test it out, you can just create a bare TypeScript app with npx create-react-app cra-typescript --typescript, then change the react-scripts dep as follows.

{
  "name": "cra-typescript",
  "version": "0.1.0",
  "private": true,
  "dependencies": {
    "@types/jest": "24.0.11",
    "@types/node": "11.13.7",
    "@types/react": "16.8.14",
    "@types/react-dom": "16.8.4",
    "react": "^16.8.6",
    "react-dom": "^16.8.6",
    "react-scripts": "fortana-co/react-scripts#master",
    "typescript": "3.4.5"
  },
  "scripts": {
    "start": "echo 'REACT_APP_ENV=dev\nTSC_COMPILE_ON_ERROR=true' > .env && react-scripts start",
    "build": "echo 'REACT_APP_ENV=prod\nTSC_COMPILE_ON_ERROR=true' > .env && react-scripts build",
    "build-stg": "echo 'REACT_APP_ENV=stg\nTSC_COMPILE_ON_ERROR=true' > .env && react-scripts build",
    "test": "react-scripts test --env=jsdom --modulePaths=src",
    "eject": "react-scripts eject"
  },
  "eslintConfig": {
    "extends": "react-app"
  },
  "browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all"
    ],
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  }
}

After this you'll be be able to run npm start and npm run build successfully, even if there are type errors. These type errors are printed to the terminal and the browser console.

…build TypeScript projects even if there are TypeScript type check errors

facebook-github-bot · 2019-04-26T20:57:25Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up 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 the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot · 2019-04-26T22:29:22Z

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

stale · 2019-05-26T23:09:17Z

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.

kylebebak · 2019-05-27T00:41:49Z

@iansu @Timer @amyrlam @ianschmitz

Hey all! The bot marked this as stale... Any chance I could get some feedback? I think the way TS errors are currently handled is a pain point that's worth fixing, and this does it in a way I think is sensible.

ignacioureta · 2019-05-30T20:46:14Z

Please consider this ticket. we have a large TS codebase and without this we can't migrate without a huge burdain

Yankovsky · 2019-07-09T09:09:18Z

The issue is a huge pain for us as well. Currently we are forced to add custom css that hides error overlay.

mrmckeb · 2019-07-15T14:30:34Z

Hi all, I have to say that I'm personally not 100% in favour of this - as I feel it could be abused. However, I also understand the concerns around migration.

As this PR solves this and still warns, I think it's OK.

I'll need a second approval though - once the issues are fixed ;)

mrmckeb · 2019-07-15T14:32:42Z

packages/react-scripts/scripts/build.js

-      process.exit(1);
+      const tscCompileOnError = process.env.TSC_COMPILE_ON_ERROR === 'true';
+      if (tscCompileOnError) {
+        console.log(chalk.yellow('Compiled with warnings.\n'));


Maybe we should state here that due to the flag, these warnings may be serious. Or just say that there were errors that the user has chosen to ignore with the flag.

I agree, we could make it more obvious that there were type errors on compilation. How about this:

console.log(chalk.red( 'Compiled with the following type errors (you may want to check these before deploying your app):\n' ));

packages/react-dev-utils/README.md

…nsole when an app is built in presence of type errors

kylebebak · 2019-07-15T17:01:45Z

@mrmckeb

Thanks for reviewing this... I think it's ready to go now!

docusaurus/docs/adding-typescript.md

kylebebak · 2019-08-19T15:39:31Z

@mrmckeb @ianschmitz

TSC_COMPILE_ON_ERROR is now documented in advanced-configuration. Is there anything else to do, or can we get this merged?

Thanks!

artalar · 2019-08-24T10:54:46Z

@ianschmitz @mrmckeb please, finish this PR, many people really need this feature.

kylebebak · 2019-09-06T21:52:21Z

@rovansteen @ianschmitz @mrmckeb

Hey guys, just bumping again. As far as I can tell this is ready to merge, and I just wanted to confirm that that's the case. If it's not, please let me know what else it needs. Thank you!

kylebebak · 2019-09-23T17:54:36Z

@rovansteen @ianschmitz @mrmckeb

Hey guys, bumping again... This PR is ready to go, and lots of people would like to see it merged.

If there's something that's preventing it from being merged it would be nice to hear what that is. If there's not, please merge it or let us know when that might happen!

RDIL · 2019-09-23T18:15:31Z

+1, please get this in asap

RDIL · 2019-09-26T18:26:18Z

@mrmckeb can we get this merged now?

mrmckeb

This looks good, I just have some small comments on the documentation.

mrmckeb · 2019-07-28T12:10:31Z

docusaurus/docs/adding-typescript.md

@@ -29,7 +29,9 @@ yarn add typescript @types/node @types/react @types/react-dom @types/jest

 Next, rename any file to be a TypeScript file (e.g. `src/index.js` to `src/index.tsx`) and **restart your development server**!

-Type errors will show up in the same console as the build one.
+Type errors will show up in the same console as the build one. By default, Create React App prevents you from running the dev server if your code has type errors. If you introduce type errors to your project, you have to fix or ignore them before you continue development.


I'd rather we omit this for now, as we don't want users to do this unless they absolutely must. Perhaps just as small snippet like "for advanced configuration, see (advanced config)" - as a terrible example.

That way, people can find it if they need, but they won't assume it's a normal practice to use this.

Ideally, this should never be used unless you're migrating a lot of legacy code.

Hey @mrmckeb

I've changed the above to the following:

Type errors will show up in the same console as the build one. You'll have to fix these type errors before you continue development or build your project. For advanced configuration, see here.

Now it doesn't explicitly or even implicitly encourage anyone to do this =)

Thanks for your feedback!

mrmckeb · 2019-08-11T09:32:31Z

docusaurus/docs/adding-typescript.md

-Type errors will show up in the same console as the build one.
+Type errors will show up in the same console as the build one. By default, Create React App prevents you from running the dev server if your code has type errors. If you introduce type errors to your project, you have to fix or ignore them before you continue development.
+
+You can remove this restriction by running your app with the `TSC_COMPILE_ON_ERROR` environment variable set to `true`, for example, by adding `TSC_COMPILE_ON_ERROR=true` as documented in our [advanced configuration](advanced-configuration.md).


I'd like to say that we don't recommend this. @ianschmitz, what are your thoughts?

Good point. I agree this is likely an advanced scenario for those porting legacy code. New code bases really shouldn't be using this option.

mrmckeb · 2019-09-30T07:29:54Z

@RDIL If you can make these last changes, I can merge this in. Thanks!

RDIL · 2019-09-30T11:19:13Z

I would love to, but this is @kylebebak 's PR, not mine!

… run dev server

kylebebak · 2019-09-30T15:10:23Z

@ianschmitz @mrmckeb @RDIL

Hey all, I made the suggested changes.

Thanks for reviewing... Looks like this is ready to go!

mrmckeb · 2019-10-01T07:54:23Z

Sorry @RDIL, I got confused between this and another PR. Multitasking fail.

Thanks @kylebebak!

mrmckeb · 2019-10-01T07:55:18Z

Trying to fix the CI.

Adds TSC_COMPILE_ON_ERROR env var to allow users to run and properly …

c1c56de

…build TypeScript projects even if there are TypeScript type check errors

kylebebak requested review from amyrlam, bugzpodder, gaearon, ianschmitz, iansu, mrmckeb, petetnt and Timer as code owners April 26, 2019 20:51

kylebebak mentioned this pull request Apr 26, 2019

Feature idea/improvement: allow dev server and build script to run in presence of TypeScript errors #6930

Closed

facebook-github-bot added the CLA Signed label Apr 26, 2019

stale bot added the stale label May 26, 2019

stale bot removed the stale label May 27, 2019

bugzpodder added this to the 3.0.2 milestone Jun 19, 2019

bugzpodder added the issue: proposal label Jun 19, 2019

bugzpodder modified the milestones: 3.0.2, 3.x, 3.1 Jun 19, 2019

mrmckeb self-assigned this Jul 15, 2019

mrmckeb suggested changes Jul 15, 2019

View reviewed changes

Documents TSC_COMPILE_ON_ERROR env var; changes message printed to co…

1955a07

…nsole when an app is built in presence of type errors

ianschmitz reviewed Aug 11, 2019

View reviewed changes

docusaurus/docs/adding-typescript.md Show resolved Hide resolved

Document TSC_COMPILE_ON_ERROR in advanced-configuration

1d86b8a

rovansteen approved these changes Aug 22, 2019

View reviewed changes

ianschmitz approved these changes Sep 23, 2019

View reviewed changes

mrmckeb reviewed Sep 27, 2019

View reviewed changes

ianschmitz requested review from ianschmitz and removed request for bugzpodder September 28, 2019 22:37

Doesn't even implicitly encourage disabling TypeScript error check to…

1bedff1

… run dev server

mrmckeb approved these changes Oct 1, 2019

View reviewed changes

mrmckeb closed this Oct 1, 2019

v3.3 automation moved this from In progress to Done Oct 1, 2019

mrmckeb reopened this Oct 1, 2019

v3.3 automation moved this from Done to In progress Oct 1, 2019

mrmckeb merged commit 71946b1 into facebook:master Oct 1, 2019

v3.3 automation moved this from In progress to Done Oct 1, 2019

ianschmitz removed this from Done in v3.3 Oct 1, 2019

ianschmitz modified the milestones: 3.2, 3.1.3 Oct 1, 2019

lock bot locked and limited conversation to collaborators Oct 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds TSC_COMPILE_ON_ERROR env var... #6931

Adds TSC_COMPILE_ON_ERROR env var... #6931

kylebebak commented Apr 26, 2019 •

edited by ianschmitz

facebook-github-bot commented Apr 26, 2019

facebook-github-bot commented Apr 26, 2019

stale bot commented May 26, 2019

kylebebak commented May 27, 2019

ignacioureta commented May 30, 2019

Yankovsky commented Jul 9, 2019

mrmckeb commented Jul 15, 2019 •

edited

mrmckeb Jul 15, 2019

kylebebak Jul 15, 2019

kylebebak commented Jul 15, 2019

kylebebak commented Aug 19, 2019

artalar commented Aug 24, 2019

kylebebak commented Sep 6, 2019

kylebebak commented Sep 23, 2019 •

edited

RDIL commented Sep 23, 2019

RDIL commented Sep 26, 2019

mrmckeb left a comment

mrmckeb Jul 28, 2019

kylebebak Sep 30, 2019

mrmckeb Aug 11, 2019

ianschmitz Sep 28, 2019

mrmckeb commented Sep 30, 2019

RDIL commented Sep 30, 2019

kylebebak commented Sep 30, 2019

mrmckeb commented Oct 1, 2019

mrmckeb commented Oct 1, 2019

Adds TSC_COMPILE_ON_ERROR env var... #6931

Adds TSC_COMPILE_ON_ERROR env var... #6931

Conversation

kylebebak commented Apr 26, 2019 • edited by ianschmitz

facebook-github-bot commented Apr 26, 2019

facebook-github-bot commented Apr 26, 2019

stale bot commented May 26, 2019

kylebebak commented May 27, 2019

ignacioureta commented May 30, 2019

Yankovsky commented Jul 9, 2019

mrmckeb commented Jul 15, 2019 • edited

mrmckeb Jul 15, 2019

Choose a reason for hiding this comment

kylebebak Jul 15, 2019

Choose a reason for hiding this comment

kylebebak commented Jul 15, 2019

kylebebak commented Aug 19, 2019

artalar commented Aug 24, 2019

kylebebak commented Sep 6, 2019

kylebebak commented Sep 23, 2019 • edited

RDIL commented Sep 23, 2019

RDIL commented Sep 26, 2019

mrmckeb left a comment

Choose a reason for hiding this comment

mrmckeb Jul 28, 2019

Choose a reason for hiding this comment

kylebebak Sep 30, 2019

Choose a reason for hiding this comment

mrmckeb Aug 11, 2019

Choose a reason for hiding this comment

ianschmitz Sep 28, 2019

Choose a reason for hiding this comment

mrmckeb commented Sep 30, 2019

RDIL commented Sep 30, 2019

kylebebak commented Sep 30, 2019

mrmckeb commented Oct 1, 2019

mrmckeb commented Oct 1, 2019

kylebebak commented Apr 26, 2019 •

edited by ianschmitz

mrmckeb commented Jul 15, 2019 •

edited

kylebebak commented Sep 23, 2019 •

edited