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

chore: Relocated tsbuildinfo out of dist folder #9390

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

Josh-Walker-GM
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM commented Nov 8, 2023

Problem
The current tsconfig entry for the tsbuildinfo file puts the file into the dist folder. This ultimately results in it being included in the shipped packages on npm. These files are large and in some cases are the bulk of the content in the package. For example in the @redwoodjs/project-config package it makes up 75% of the package size.

From what I can read online, and my own understanding of the functionality of that file, there is no need for this content to be shipped with the built code on npm.

I quickly checked on my local machine and this change does not appear to have impacted the build time.

Change

  1. Delete the explicit entry which will place it in a default location which is fine. The file is already ignored by the existing .gitignore.
  2. Remove the code in the release tooling that removes these files. I don't think it was fully functional right now.

Notes
I marked this as a chore because it doesn't influence the code just the build step. It does change the bytes that are shipped to users so maybe it's not a chore but I am not that concerned with the label right now.

@Josh-Walker-GM Josh-Walker-GM added the release:chore This PR is a chore (means nothing for users) label Nov 8, 2023
@Josh-Walker-GM Josh-Walker-GM added this to the chore milestone Nov 8, 2023
@Josh-Walker-GM Josh-Walker-GM self-assigned this Nov 8, 2023
@Josh-Walker-GM Josh-Walker-GM added the fixture-ok Override the test project fixture check label Nov 8, 2023
Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Josh-Walker-GM! Yeah the code in the release command never worked because lerna would build all the packages before publishing them and add the files back. One question on studio but otherwise excited to release this.

packages/studio/tsconfig.json Show resolved Hide resolved
@jtoar
Copy link
Contributor

jtoar commented Nov 9, 2023

@Josh-Walker-GM looking at the docs here: https://www.typescriptlang.org/tsconfig#tsBuildInfoFile. It says for outDir, which most of our packages specify:

If outDir is set, then the default is <outDir>/<config name>.tsbuildInfo

But with your PR, I do indeed see it in <packageDir>, not <outDir>. Any idea why? Something to do with the fact that we don't use tsc to build but just to type check maybe?

@jtoar
Copy link
Contributor

jtoar commented Nov 9, 2023

Sorry should've read those docs more carefully before asking, we specify both rootDir and outDir which leads to different behavior:

If rootDir and outDir are set, then the file is <outDir>/<relative path to config from rootDir>/<config name>.tsbuildinfo For example, if rootDir is src, outDir is dest, and the config is ./tsconfig.json, then the default is ./tsconfig.tsbuildinfo as the relative path from src/ to ./tsconfig.json is ../.

@Josh-Walker-GM
Copy link
Collaborator Author

Exactly, I think it's because of the root and dist dir combo behaviour.

@Josh-Walker-GM Josh-Walker-GM enabled auto-merge (squash) November 9, 2023 00:35
@Josh-Walker-GM Josh-Walker-GM merged commit cdfa692 into main Nov 9, 2023
33 checks passed
@Josh-Walker-GM Josh-Walker-GM deleted the jgmw-chore/tsbuildinfo branch November 9, 2023 00:54
@jtoar jtoar modified the milestones: chore, next-release Nov 9, 2023
jtoar pushed a commit that referenced this pull request Nov 9, 2023
**Problem**
The current tsconfig entry for the tsbuildinfo file puts the file into
the dist folder. This ultimately results in it being included in the
shipped packages on npm. These files are large and in some cases are the
bulk of the content in the package. For example in the
`@redwoodjs/project-config` package it makes up 75% of the package size.

From what I can read online, and my own understanding of the
functionality of that file, there is no need for this content to be
shipped with the built code on npm.

I quickly checked on my local machine and this change does not appear to
have impacted the build time.

**Change**
1. Delete the explicit entry which will place it in a default location
which is fine. The file is already ignored by the existing .gitignore.
2. Remove the code in the release tooling that removes these files. I
don't think it was fully functional right now.

**Notes**
I marked this as a chore because it doesn't influence the code just the
build step. It does change the bytes that are shipped to users so maybe
it's not a chore but I am not that concerned with the label right now.
@thedavidprice
Copy link
Contributor

Wow... 🤯

Nice to have a substantial, easy win every one in awhile. Nice work Josh! 🚀

dac09 added a commit to dac09/redwood that referenced this pull request Nov 9, 2023
…nto fix/stream-wait-allready

* 'fix/stream-wait-allready' of github.com:dac09/redwood:
  chore: Relocated tsbuildinfo out of dist folder (redwoodjs#9390)
  chore(deps): update dependency lerna to v7.4.2 (redwoodjs#9380)
@Josh-Walker-GM
Copy link
Collaborator Author

Thanks but it was definitely @jtoar that spotted this. I just deleted the lines haha!

@jtoar jtoar modified the milestones: next-release, v6.4.0 Nov 11, 2023
jtoar added a commit that referenced this pull request Nov 13, 2023
Follow up to #9390. Previously,
framework sync would remove all of a package's dist, then rebuild it.
This meant the `tsconfig.tsbuildinfo` was removed too.

#9390 moved the `tsconfig.tsbuildinfo`, saving megabytes of space for
user's projects, but didn't update `yarn build:clean` or `yarn rwfw
project:sync`. This PR restores their behavior. Paired with
@Josh-Walker-GM on this one.
jtoar added a commit that referenced this pull request Nov 16, 2023
Follow up to #9390. Previously,
framework sync would remove all of a package's dist, then rebuild it.
This meant the `tsconfig.tsbuildinfo` was removed too.

#9390 moved the `tsconfig.tsbuildinfo`, saving megabytes of space for
user's projects, but didn't update `yarn build:clean` or `yarn rwfw
project:sync`. This PR restores their behavior. Paired with
@Josh-Walker-GM on this one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixture-ok Override the test project fixture check release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants