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

refactor: typescript projects & docs generation #2804

Merged
merged 26 commits into from
May 3, 2022

Conversation

clavin
Copy link
Member

@clavin clavin commented Apr 15, 2022

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:
As a precursor to getting docs automatically generated and pushed, this PR refactors how the typescript projects are set up & how docs are generated.

⚠️ With this setup, one part of the docs gen takes >4GB of memory. I'm not quite sure why typedoc needs that much memory, but it does. I was able to get around this by increasing the memory Node.js is allowed to use.

"build:quick": "bolt ws exec -- swc src --out-dir dist --quiet --extensions \".ts\" --config-file ../../../.swcrc",
"clean": "rimraf dist && bolt ws exec -- rimraf dist tsconfig.tsbuildinfo",
"build": "bolt ws exec -- swc src --out-dir dist --quiet --extensions \".ts\" --config-file ../../../.swcrc",
"build:full": "bolt ws exec -- tsc -b",
Copy link
Member Author

@clavin clavin Apr 15, 2022

Choose a reason for hiding this comment

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

I changed the set of build commands from build:quick and build (which just calls the former) to build (same as build:quick before) and build:full which makes a full typescript build, along with generated types, which I think should be in the distributed packages.

@codecov
Copy link

codecov bot commented Apr 21, 2022

Codecov Report

Merging #2804 (1f056b0) into master (9fcd08f) will decrease coverage by 2.92%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2804      +/-   ##
==========================================
- Coverage   75.62%   72.69%   -2.93%     
==========================================
  Files          77       77              
  Lines        2359     2344      -15     
  Branches      438      442       +4     
==========================================
- Hits         1784     1704      -80     
+ Misses        462      419      -43     
- Partials      113      221     +108     
Impacted Files Coverage Δ
packages/installer/darwin/src/InstallerDarwin.ts 35.00% <0.00%> (ø)
packages/api/core/src/util/parse-archs.ts 60.00% <0.00%> (-40.00%) ⬇️
packages/api/core/src/util/out-dir.ts 66.66% <0.00%> (-22.23%) ⬇️
packages/api/core/src/util/hook.ts 75.00% <0.00%> (-15.00%) ⬇️
packages/api/core/src/util/forge-config.ts 80.59% <0.00%> (-14.93%) ⬇️
packages/api/core/src/util/electron-version.ts 75.38% <0.00%> (-13.85%) ⬇️
packages/api/core/src/util/install-dependencies.ts 79.31% <0.00%> (-13.80%) ⬇️
packages/api/core/src/api/install.ts 75.00% <0.00%> (-13.16%) ⬇️
packages/api/core/src/api/publish.ts 68.42% <0.00%> (-11.85%) ⬇️
packages/api/core/src/api/start.ts 65.88% <0.00%> (-11.77%) ⬇️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fcd08f...1f056b0. Read the comment docs.

@clavin clavin marked this pull request as ready for review April 21, 2022 19:36
clavin and others added 2 commits April 22, 2022 11:33
Co-authored-by: Calvin <clavin@users.noreply.github.com>
@erickzhao erickzhao self-requested a review May 2, 2022 20:52
.github/workflows/ci.yml Outdated Show resolved Hide resolved
* This script creates `index.ts` files in each package directory that simply
* re-export the exports of the real main file. This is necessary for when all
* the packages are run under a common context (instead of each individually).
* This is because the symlinks bolt makes for each package are descendants of
Copy link
Member

@erickzhao erickzhao May 2, 2022

Choose a reason for hiding this comment

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

question(non-blocking): Interesting. So in theory when we move away from bolt in the future this would be unnecessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think other tools will still have this issue. The issue stems from when we run a command on "all packages" (run from the project root and have it traverse into everything including the packages), which is in contrast to running a command "per-package" (running the command in every package with that package as the root).

This difference is significant because if you start in root/ and traverse down to root/packages/my-package/node_modules/symlink-to-my-other-package (a symlink) and that takes you to root/packages/my-other-package, you're still under the root context root/ which means that tooling will not look for a package.json file because it does not think you're checking a dependency. That is why the index file solution works, because the index file will be automatically checked when the dependency resolution thinks it's still under the root context (which is true because we chose the root to be the project, rather than a specific package).

Does that make more sense?

@erickzhao erickzhao merged commit 412327a into master May 3, 2022
@erickzhao erickzhao deleted the clavin/fix-docs-deploy branch May 3, 2022 17:29
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.

2 participants