Skip to content

support for custom tsconfig through TSCONFIG_PROD and TSCONFIG_DEV#11031

Merged
CarlRibbegaardh merged 0 commit into
react:mainfrom
CarlRibbegaardh:master
Aug 23, 2022
Merged

support for custom tsconfig through TSCONFIG_PROD and TSCONFIG_DEV#11031
CarlRibbegaardh merged 0 commit into
react:mainfrom
CarlRibbegaardh:master

Conversation

@CarlRibbegaardh
Copy link
Copy Markdown

I've added 2 environment variables. TSCONFIG_BUILD and TSCONFIG_WATCH.
They are used for setting a custom tsconfig in a monorepo setting.

The reason for this is when a tsconfig.json inherits a base tsconfig with all interrnal package folders defined in order to facilitate easy navigation from the outermost projects into components inside library projects, it causes the build process to navigate through the source files instead of the build/dist folders of the library project builds. This is usually solved by updating the tsconfig on the fly before react-scripts build or copying between build and edit mode configs.

I've used env-cmd with the following setting for building.

TSCONFIG_BUILD=tsconfig.build.json
TSCONFIG_WATCH=tsconfig.watch.json

@facebook-github-bot
Copy link
Copy Markdown

Hi @CarlRibbegaardh!

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 cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Copy Markdown

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

@mrmckeb
Copy link
Copy Markdown
Contributor

mrmckeb commented May 31, 2021

Hi @CarlRibbegaardh, I think I understand the issue you want to resolve here. Could you provide some more details about how your monorepo works with CRA? I assume that all of these packages have independent build processes?

Also, if we do move forward with this, I think the appropriate names would be .dev and .prod (or similar).

@mrmckeb mrmckeb self-assigned this May 31, 2021
@CarlRibbegaardh
Copy link
Copy Markdown
Author

CarlRibbegaardh commented May 31, 2021

Hi @CarlRibbegaardh, I think I understand the issue you want to resolve here. Could you provide some more details about how your monorepo works with CRA? I assume that all of these packages have independent build processes?

Also, if we do move forward with this, I think the appropriate names would be .dev and .prod (or similar).

The current solution I'm working at has 2 prior libraries that is simply ts compiled. It has one recently added library using rollup which was created using the cra typescript as a template and then modified to use rollup. It has two frontend apps using cra 4 typescript template.

The two first libraries core and components is compiled using tsc to a build folder. The foundation library using rollup compiles to a dist folder. The frontends have dependencies to the libraries.
In order to get a nice editing experience in VSCode, we have a base tsconfig.json which sets up the baseUrl to point to packages and points all our internal references as paths to their respective source folders.
That way, we can navigate directly from frontend app to library code in the editor.
However, using only one tsconfig.json for each frontend, causes the build process to consider the source of the libraries instead of the build and the dist folders. So by having a separate tsconfig without baseUrl and paths, the build correctly reference the local libraries through links in node_modules, find their package.json and can correctly resolve their typings and js files.

In order to have multiple configs, we've been editing them on the fly and restoring them. But ever so often the config files are changed and we need to revert them before committing. It's a troublesome procedure.
With the possibility to point out other tsconfig files, the building and editing becomes much more streamlined.

I have a temporary npm package here with some more documentation:
https://www.npmjs.com/package/carlribbegaardh-react-scripts

@CarlRibbegaardh
Copy link
Copy Markdown
Author

Also, if we do move forward with this, I think the appropriate names would be .dev and .prod (or similar).

I've gone through all changes and updated so that dev and prod are the terms used throughout the feature.

@CarlRibbegaardh CarlRibbegaardh changed the title support for custom tsconfig through TSCONFIG_BUILD and TSCONFIG_WATCH support for custom tsconfig through TSCONFIG_PROD and TSCONFIG_DEV May 31, 2021
@CarlRibbegaardh
Copy link
Copy Markdown
Author

@mrmckeb I hope you have seen the details and changes provided so far. Does it paint a clear picture or am I too vague?

@CarlRibbegaardh
Copy link
Copy Markdown
Author

@ianschmitz @iansu @petetnt Any chance of any feedback or discussion of this PR? I'm not that used to github so I'm sorry if there's a better way to connect that I should have used.

@CarlRibbegaardh
Copy link
Copy Markdown
Author

@mrmckeb @ianschmitz @iansu @petetnt
I'm afraid this issue might be forgotten, so here's some food for thought.

It would help a lot with a way to point to a tsconfig. :)

@CarlRibbegaardh
Copy link
Copy Markdown
Author

Bumping again, hoping for feedback.

@stale
Copy link
Copy Markdown

stale Bot commented Jan 9, 2022

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.

@stale stale Bot added the stale label Jan 9, 2022
@MartinHarkins
Copy link
Copy Markdown

Hey there, I know this is stale but this sort of feature is desirable. And the proposal would go along a ways.

As mentionned in #6023 (comment)

Support for multiple tsconfigs is a fundamental requirement for any build tool that claims to support TypeScript. Not having it is the real footgun.

The reason is that, in order for intellisense in editors like VSCode to work, your root tsconfig.json must include every source file and dependency in your app. I.e. all your app's source files, all the .test.ts files, the .stories.tsx files, etc. And the types/libs must reference things like Storybook, node, Jest, etc. as well.

However, when building your app, you obviously don't want to include everything. In fact, it is very dangerous to do so. If one of your app's components accidentally imported something from Storybook or node, you wouldn't get an error and build/type-check time. Therefore, you need to create a separate tsconfig.app.json that extends from the root tsconfig.json but which narrows the list of included files and allowed dependencies. You create a similar tsconfig file for your tests, Storybook, Cypress, etc.

This is how other tools like Angular CLI work, and it is how it is supposed to work. It's the whole reason TypeScript supports multiple tsconfig files and extending from each other.

Thanks guys :)

@stale stale Bot removed the stale label Jan 14, 2022
@CarlRibbegaardh CarlRibbegaardh merged commit f34d88e into react:main Aug 23, 2022
@CarlRibbegaardh
Copy link
Copy Markdown
Author

What happened here? I synchronized the current master to my repo and then suddenly something happens here? Not good.

@fabien-somnier
Copy link
Copy Markdown

@CarlRibbegaardh it seems that you've somehow force-pushed facebook:master into you CarlRibbegaardh:master, and lost you branch code changes (making this PR automatically closed itself as there's nothing to be merged anymore here)

@CarlRibbegaardh
Copy link
Copy Markdown
Author

That's ok. I was afraid I managed to force push things to facebook:master. Thanks for checking!

@fabien-somnier
Copy link
Copy Markdown

@CarlRibbegaardh but didn't you previously suggest some changes in this PR, that have then been lost? or is TSCONFIG_PROD/TSCONFIG_DEV supported now already?

@CarlRibbegaardh
Copy link
Copy Markdown
Author

@fabien-somnier
Yes, but there was some changes in these files, so I did some local branches and afterwards synchronized the code (a github button). That's when this one got cleared out. So I still have the local changes.
But I was applying them and looked through the code and there was some stuff in "modules" that made me unsure. That code doesn't know the state of develop vs prod, and it uses the config file. I think I would need some more insight in how that works to be sure that my suggestion works with the current version of CRA.

@CarlRibbegaardh
Copy link
Copy Markdown
Author

@fabien-somnier @MartinHarkins @mrmckeb @ianschmitz @iansu @petetnt @grandsilence @vicrep @TomaszG @dylnclrk @jboler
I have updated this to work with CRA 5.0. New PR and sample here #12711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants