Workspaces typescript fixes

[pmoleri]

Hi there. I created this PR with some changes for typescript monorepos, it's working fine for me after I add the "main:src" entries in the leaf package.json files.

[F1LT3R]

Wow, this is fantastic! Thanks.

I will look into this on Monday.

[F1LT3R]

Wondering why you thought to remove this?

My best guess is that you didn't see it doing anything for your project, and that you were thinking it was not needed?

Hopefully it was not breaking anything for you in Typescript?

The reason I added, this was so the dev-server does not lint workspace dependencies that were not added to the package.json file of the app that is being launched. I think you'll find that without these lines, all of your worspaces with main:src in the package.json will be linted when launching your React app from the dev-server, even if the dependencies never get used by the app.

The way we are using react-workspaces we currently have 7 apps, and 13 suites of components. Linting 13 suites of components for 7 apps takes a couple minutes; but it takes about 10 seconds for one app. As the monorepo grows, linting unused code gets increasingly expensive, so we would like to avoid this cost.

Interested to get your thoughts.

[pmoleri]

Hi, thanks for explaining the rationale, I think watch and linting are working as expected.
I just removed them because they seem to be currently unused in the in master. I only noticed it because VS Code shows them grayed out.

Reviewing the code again... I'll make my own guess :)
I think the functions were renamed and both versions were kept:

filterDeps -> filterSrcPaths 
filterDepsTable -> buildDepsTable 

and old versions can be safeley removed.
Does that seem right?

[F1LT3R]

I think I see what you mean. I will double check this. Thanks!

[Driox]

This PR is really mandatory for typescript, especially Disable typescript linting outside main project
Please merge it quickly

[pmoleri]

Hi @Driox , this PR has been working for me, but now a hit an issue. I'd like to know if you have any idea on how to solve it.

This situation works fine when I start app-a:

packages/comp-1/package.json  { "main:src": "src/index.ts", "main": "src/index.ts" }
packages/app-a/package.json  { dependencies: { "comp-1": "*" } }

Now I want to publish comp-1 as a package, so I change "main": "src/index.ts" -> "main": "dist/index.js". Now I get compilation errors from app-a.
Why? fork-ts-checker-webpack-plugin doesn't use webpack config, but native typescript module resolution, so it ignores the main:src field and it just uses "main" field, but I don't want to use compiled code in comp-1 I want the compiler to be able to pickup source code.

I tried:
// tsconfig.json

  "baseUrl": ".",
  "paths": {
    "comp-1": ["../comp-1/src/index.ts"]
  },

to no avail.

Any idea on how to solve this?

[Driox]

I think you should split this PR in multiple PR that can be merged independently

For instance, I cherry pick this commit and it allows me to copy / paste my existing typescript project without issue

e78111b

I don't know if this project is useful for component that mean to be published.
I use it to reuse react code across multiple react applications. I publish the applications, not the components. And when I say publish, it's more release build in our private company, not publish on npm

[pmoleri]

Great. It's good that someone else validated the result.
The last 2 commits are the ones that I are specific to typescript, but I think that all commits are improvements and safe to merge, for example my project cannot import some 3rd party packages without commit 2dec991, so for me it was essential to test the result. I think socket.io-client was one of those packages.

@F1LT3R Do you think we can merge this as it is or you'd rather split it?

[F1LT3R]

I use it to reuse react code across multiple react applications. I publish the applications, not the components. And when I say publish, it's more release build in our private company, not publish on npm

This is also how we are using it.

[F1LT3R]

@F1LT3R Do you think we can merge this as it is or you'd rather split it?

I will try and look tomorrow and merge.

[Toub]

Awesome!

[vstlouis]

Status update on this?

[proof666]

I tried this PR and works like charm!

Whats plan on it?

[priley86]

Very nice work here! I have cloned this branch and merged w/ the latest react-scripts upstream.
https://github.com/priley86/create-react-app/commits/master

My sample test repo is here:
https://github.com/priley86/react-workspaces-playground

I'm quite happy w/ this. I've also tested w/ typescript in another repo. 👏

My only question is now... what should the "component" project's "build" script be set to?
https://github.com/priley86/react-workspaces-playground/blob/master/packages/components/package.json#L8

This script will currently give an error (below). Should it be removed since we are setting "main": "src/index.js" and the application package just imports and builds the source of those libraries directly? Eventually, it may be nice to also build those into a dist folder and expose as compiled CJS/ESM modules so that others may consume those packages directly. This meets my usecase for now though, so I may just remove it temporarily... just curious if you had any suggestions?

| ~/Github/react-workspaces-playground/packages/components @ PARILEY2-M-47FB (pariley2) 
| => yarn build
yarn run v1.19.1
$ react-scripts build
Yarn Workspaces paths detected.
Found 2 path(s) with "main:src" entry.
Exporting Workspaces config to Webpack.
{ root: '/Users/pariley2/Github/react-workspaces-playground',
  paths: [],
  packageEntry: 'main:src',
  development: true,
  production: true }
Could not find a required file.
  Name: index.html
  Searched in: /Users/pariley2/Github/react-workspaces-playground/packages/components/public
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Command failed: /Users/pariley2/.yvm/versions/v1.19.1/bin/yarn.js build

Really appreciate this work here and may contribute updates if I have bandwidth!

[abrkn]

Any progress on this?