Skip to content
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

Tooling updates #420

Merged
merged 4 commits into from
Oct 21, 2023
Merged

Tooling updates #420

merged 4 commits into from
Oct 21, 2023

Conversation

lauckhart
Copy link
Collaborator

  • Use --isolatedModules as using a non-tsc transpiler is a little unsafe without it
  • Use --skipLibCheck as type checking libraries is largely redundant
  • Invoke typescript compiler via API to avoid warmup cost
  • Fix type checking for tests which was inadvertently disabled by tsconfig restructure
  • Stop using tsconfig entirely for command line builds except for type references
  • Only emit types for libraries, just validate for applications
  • New "graph" build command to display workspace dependency graph
  • New "--workspaces" build option removes NPM and TSC overhead from workspace build
  • On rebuild skip packages with no changes to source files or dependencies
  • More robust bootstrapping for build & run
  • Improved modularity of tooling code, mostly to support workspace builds
  • Fix various relative path resolution issues when using --prefix
  • Infrastructure is in place for parallel build but didn't bother as it'd require UI work

With --isolatedModules TS chokes on these.  Esbuild ignores the "const" part
so the constness is irrelevant anyway.
@lauckhart lauckhart force-pushed the more-speedier branch 2 times, most recently from d6f2d25 to f50fac7 Compare October 19, 2023 18:56
@Apollon77
Copy link
Collaborator

@vilic if you like ... be invited for review too ... you also seem to know a bit on Typescript and such :-)

@lauckhart lauckhart force-pushed the more-speedier branch 4 times, most recently from 599e5fc to 48384a2 Compare October 20, 2023 03:53
- Use --isolatedModules as using a non-tsc transpiler is a little unsafe without it
- Use --skipLibCheck as type checking libraries is largely redundant
- Invoke typescript compiler via API to avoid warmup cost
- Fix type checking for tests which was inadvertently disabled by tsconfig restructure
- Stop using tsconfig entirely for command line builds except for type references
- Only emit types for libraries, just validate for applications
- New "graph" build command to display workspace dependency graph
- New "--workspaces" build option removes NPM and TSC overhead from workspace build
- On rebuild skip packages with no changes to source files or dependencies
- More robust bootstrapping for build & run
- Improved modularity of tooling code, mostly to support workspace builds
- Fix various relative path resolution issues when using --prefix
- Infrastructure is in place for parallel build but didn't bother as it'd require UI work
@lauckhart lauckhart force-pushed the more-speedier branch 2 times, most recently from 77077de to 652b408 Compare October 20, 2023 05:28
@vilicvane
Copy link
Contributor

@vilic if you like ... be invited for review too ... you also seem to know a bit on Typescript and such :-)

Thanks for the invitation. I looked through the changed files, but honestly this PR is both overwhelming and hard core to me. As the workflow doesn't seem familiar, and I never used many APIs appears in this PR.

But to the part I understand, it looks good to me. Actually I considered to replace all const enum to enum in my previous PR, but thought there would be too many changes. As they are now replaced, I suggest adding a lint rule banning the usage of const enum.

Some other suggestions/questions related:

  1. Why not using .prettierignore instead of list of patterns? VSCode extension automatically formats everything that's not in the ignore list anyway. Which may result in some unnecessary changes in a PR polluting the line history.
  2. skipLibCheck is good, but that also skips .d.ts in source code. Need a convention that bans .d.ts as source code.
  3. I genuinely wonder how do you watch the build? Or you guys just don't do that, at least for now?

@Apollon77
Copy link
Collaborator

I can tell for 3.)
Because I most need to work with matter.js in general and matter-node or shell or such as a secondry target this is all not really making sense with watch (We still did not found out how to have a "sub project" (matter-node.js) also use the ts stuff and such ... so I do not use "watch" in this case here.

@Apollon77
Copy link
Collaborator

PS: Thank you very much for your time to check it. I totally agree to your first paragraph ;-)) "Black magic" :-)

@lauckhart
Copy link
Collaborator Author

Thanks @vilic You're right, .prettierignore seems like a better way to go. I'll make the change next time I have to touch the whitelist if nobody does it first. Also good point on .d.ts files, maybe a custom eslint rule is in order.

I don't really watch either, between large refactors and code gen I seem to have the world blown up a lot. I've used nodemon a couple of times but mostly just have esbuild and/or npm build pipelined before run/debug.

I considered adding a watch script to the root package.json using nodemon and our "build" but figured if we're going to maintain your tsconfig structure then tsc build/watch might be preferrable.

@vilicvane
Copy link
Contributor

@lauckhart You may also consider that for eslint as well. BTW single quote (related pattern lists in package.json scripts) is not compatible on Windows (cmd) either.

@lauckhart
Copy link
Collaborator Author

BTW single quote (related pattern lists in package.json scripts) is not compatible on Windows (cmd) either.

Ah right, that raises the priority on moving to dotfiles. I opened #425 to track.

@lauckhart lauckhart merged commit 834f63e into project-chip:main Oct 21, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants