Skip to content

Automatically generate and validate tsconfig.json #5508

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

Closed
2 tasks
Timer opened this issue Oct 21, 2018 · 21 comments
Closed
2 tasks

Automatically generate and validate tsconfig.json #5508

Timer opened this issue Oct 21, 2018 · 21 comments
Milestone

Comments

@Timer
Copy link
Contributor

Timer commented Oct 21, 2018

When using TypeScript, we should automatically generate and validate tsconfig.json.

  • force isolatedModules: true
  • remove .d.ts files from template
@Timer Timer added this to the 2.1 milestone Oct 21, 2018
@ianschmitz
Copy link
Contributor

ianschmitz commented Oct 22, 2018

I can take this on @Timer. A few thoughts:

We could publish the src/loaders.d.ts contents as @types/react-scripts, which would be picked up automatically by TypeScript. This would also potentially benefit non-TypeScript users of CRA, as they would get some nice typings within VSCode for things like *.svg, *.module.css, etc.

What do you think?

EDIT: Sorry i think GitHub is having issues. I didn't think my original comments got posted. I deleted them in place of this one.

@ianschmitz ianschmitz mentioned this issue Oct 22, 2018
2 tasks
@Timer
Copy link
Contributor Author

Timer commented Oct 22, 2018

Hey @ianschmitz I never saw this comment until now since GH was having issues all evening 😅. I believe we overlapped on some work -- let's see what we can do about incorporating changes from both of them.

@Timer Timer closed this as completed Oct 22, 2018
@ianschmitz
Copy link
Contributor

No worries. What are your thoughts on the @types package?

@Timer
Copy link
Contributor Author

Timer commented Oct 22, 2018

I'm not sure that's the best thing to do, it's very common to not be on the latest react-scripts and keeping the two in sync could become problematic. Right now the toolbox copies it's declaration's file into your project. I think this is OK.

Alternatively, can we add an entry to tsconfig that references something like react-scripts/declarations?

@ianschmitz
Copy link
Contributor

ianschmitz commented Oct 22, 2018

Yeah that would work. I was initially thinking that we could have a dependency in react-scripts that depends on @types/react-scripts, so we would control both sides during releases.

However your idea would probably keep things simpler and be one less package to manage. It just means a little less benefit for non TypeScript users.

@Timer
Copy link
Contributor Author

Timer commented Oct 22, 2018

It just means a little less benefit for non TypeScript users.

What do you mean by this?

@ianschmitz
Copy link
Contributor

ianschmitz commented Oct 22, 2018

I think VS Code by default will pick up types in your node_modules/@types directory, which will give some IntelliSense in .js files to help you out. You can also add a // @ts-check comment in your .js files to get some inline type checking (it underlines errors in red, etc). So if we depended on an @types package (@types/react-scripts), it should be picked up and used automatically in javascript projects, as well as TypeScript projects 😃.

@ArcticZeroo
Copy link

ArcticZeroo commented Dec 20, 2018

Why was this decision made? The overwriting of my tsconfig is causing tsc to do literally nothing (even with plainly broken code or large code changes it doesn't recompile or error). When I change the include from src to ./src/* (before I had react-scripts 2.x it had no include at all and worked fine...), it starts actually doing stuff but all my custom types break and when I try to add typeRoots it's overridden. Seems like really unfriendly behavior.

There should be a way to opt out of this behavior without ejecting. Seems pretty overzealous

@Timer
Copy link
Contributor Author

Timer commented Dec 20, 2018

@ArcticZeroo can you please share your full tsconfig.json? That sounds like a TypeScript bug -- src definitely works and shouldn't be any different than ./src/*.

@ArcticZeroo
Copy link

ArcticZeroo commented Dec 20, 2018

@Timer

Sure,
Here's the config that I was using before react-scripts 2.x (the one which works perfectly fine):

{
  "compilerOptions": {
    "target": "es5",
    "sourceMap": true,
    "lib": [ "es2018", "dom" ],
    "jsx": "react",
    "downlevelIteration": true,
    "noImplicitAny": true,
    "esModuleInterop": true
  },
  "exclude": [
    "node_modules"
  ]
}

Here's the one react-scripts really wants me to use for some reason (this one does literally nothing when I type tsc):

{
  "compilerOptions": {
    "target": "es5",
    "sourceMap": true,
    "lib": [
      "es2018",
      "dom"
    ],
    "jsx": "preserve",
    "downlevelIteration": true,
    "noImplicitAny": true,
    "esModuleInterop": true,
    "allowJs": true,
    "skipLibCheck": false,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true
  },
  "exclude": [
    "node_modules"
  ],
  "include": [
    "src"
  ]
}

And this one actually does try to compile the files, but completely ignores all my custom types in src/types and fails to compile as a result (only difference is the src include)

{
  "compilerOptions": {
    "target": "es5",
    "sourceMap": true,
    "lib": [
      "es2018",
      "dom"
    ],
    "jsx": "preserve",
    "downlevelIteration": true,
    "noImplicitAny": true,
    "esModuleInterop": true,
    "allowJs": true,
    "skipLibCheck": false,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true
  },
  "exclude": [
    "node_modules"
  ],
  "include": [
    "./src/*"
  ]
}

@ArcticZeroo
Copy link

Would it just be possible to disable React's TS support for 2.x? I can (and would prefer to) compile the files myself instead

@Timer
Copy link
Contributor Author

Timer commented Dec 20, 2018

It appears that noEmit is set to true, so no files will be outputted. It should only respond with type errors.

@ArcticZeroo
Copy link

ArcticZeroo commented Dec 20, 2018

That seems really weird to be a default for setting up TS 😕 I definitely didn't set that one. Compilation does work OK with that removed. Still come compilation problems, investigating...

Is there any reason that jsx is also set to preserve ? That's causing my index.tsx to get compiled to index.jsx instead of index.js, which causes errors from it not being found. I had it set to react before

Now it's complaining about ordering of imports...

./src/util/UrlUtil.js
  Line 11:  Import in body of module; reorder to top  import/first

Search for the keywords to learn more about each error.

It's not really something I can control since the TS compiler is moving the import down...

@ArcticZeroo
Copy link

Been looking into it more and I still don't understand the decisions behind most of these config changes. With webpack (plus the fact that TS can change import to require) there's no reason to make the module type esnext, it can easily be es2015 (and then it passes the unnecessary ESLint checks). It also doesn't make any sense to have noEmit set to true (when I set it to false it goes back to true!) since completely restarting the react app still doesn't cause create-react-app to recompile the TS, so if I have to compile it myself why would it be disabled? This seems silly.

@Timer
Copy link
Contributor Author

Timer commented Dec 21, 2018

We don't support you having TypeScript files side-by-side to JavaScript files. You either need to let Create React App compile TypeScript itself, or move your project files to a different folder.
The options set are required because that's how our integration works -- you seem to be fighting against it instead of using it.

@ArcticZeroo
Copy link

I assumed that was the case, so I deleted all the .js, .jsx, etc files from the src/ folder (so I ONLY had typescript) but it was complaining about missing src/index.js specifically, which makes it difficult to work with the react app compiling for me :(

@Timer
Copy link
Contributor Author

Timer commented Dec 21, 2018

@ArcticZeroo please delete all JavaScript files in src/ (be sure you have them saved/backed up elsewhere), delete your tsconfig.json, and re-run npm start.

Next, cancel your dev server and update your tsconfig.json, only changing lib back to [ "es2018", "dom" ]. You can optionally set skipLibCheck to true.

Start your dev server again, did that work?

@ArcticZeroo
Copy link

@Timer a tentative yes, it seems like it is working now! I'm not sure why it was expecting JS before since I have no other JS at all in my project and it was giving Typescript errors as well.

Thanks! I promise I wasn't trying to be difficult here, just trying to get my project building :)

@Timer
Copy link
Contributor Author

Timer commented Dec 21, 2018

No worries, if this is something we can improve in our docs that'd be great!

@ArcticZeroo
Copy link

Yeah, that would definitely be nice. Unless I am just bad at googling, I couldn't actually find any information at all about TS support in React aside from the blog post (and ended up just having to find relevant commits to adding the TS support to figure out what's going on) -- is there any kind of doc about it to start on? I'd be willing to write something up to go over the issues I faced migrating from 1.x to 2.1.x :)

@Timer
Copy link
Contributor Author

Timer commented Dec 23, 2018

There's no documentation right now except "Adding TypeScript" -- if you could propose something that'd be great, we're open to hearing any feedback.

@lock lock bot locked and limited conversation to collaborators Jan 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants