-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
refactor: rename tsconfig.build files in examples #2201
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
3fc24f7
to
037896d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just wondering...why we named them as tsconfig.build.json
in the first place?
cc @bajtos @raymondfeng may know the answer, thanks!
I believe this change is going to break "refactor - rename" and "go to definition" commands in VS Code. Let me explain the background. It would be great if you could capture this information in our docs, e.g. DEVELOPING.md. Right now, our monorepo has two TypeScript setups in place.
That's why we have two sets of
@raymondfeng and I are looking into ways how to leverage TypeScript's Project References to allow us to have the same tsconfig for both build & VSCode, but that's a longer term work. See #1636. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As explained in my previous commit, this change is very likely to break VSCode experience inside our monorepo.
I am proposing to use a different approach: when unpacking the example from npm tarball, rename tsconfig.build.json
to tsconfig.json
. See the following two places in our codebase to get you started:
037896d
to
23c4adf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one minor comment.
f5b8637
to
0481f81
Compare
Co-authored-by: Miroslav Bajtoš <mbajtoss@gmail.com>
0481f81
to
c77e332
Compare
Connect to #2175.
Renamed the
tsconfig.build.ts
files in the examples totsconfig.ts
because, when using the examples from thelb4 example
command, they weren't extendingtsconfig.common.json
until after the rename. They also now match therpc-server
example.Checklist
npm test
passes on your machinepackages/cli
were updatedexamples/*
were updated