nodejs.org icon indicating copy to clipboard operation
nodejs.org copied to clipboard

build(partners): treeshake partners

Open bjohansebas opened this issue 2 months ago • 5 comments

Description

This doesn’t reduce the build, at least locally. Any other ideas on how to reduce the build size because of the logos?

Validation

Related Issues

Check List

  • [ ] I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • [ ] I have run pnpm format to ensure the code follows the style guide.
  • [ ] I have run pnpm test to check if all tests are passing.
  • [ ] I have run pnpm build to check if the website builds without errors.
  • [ ] I've covered new added functionality with unit tests if necessary.

bjohansebas avatar Oct 25 '25 19:10 bjohansebas

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
nodejs-org Ready Ready Preview Oct 25, 2025 7:56pm

vercel[bot] avatar Oct 25 '25 19:10 vercel[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 76.57%. Comparing base (5d333e8) to head (9c2646c). :white_check_mark: All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8265      +/-   ##
==========================================
+ Coverage   76.56%   76.57%   +0.01%     
==========================================
  Files         117      117              
  Lines        9733     9733              
  Branches      329      329              
==========================================
+ Hits         7452     7453       +1     
+ Misses       2279     2278       -1     
  Partials        2        2              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 25 '25 19:10 codecov[bot]

Could we move all the SVGs to the public folder and use the Image component to import them?

bjohansebas avatar Oct 25 '25 20:10 bjohansebas

Could we move all the SVGs to the public folder and use the Image component to import them?

That would introduce another set of problems. The reason why your code is not treeshaked is because you're using * wildcard imports (https://github.com/nodejs/nodejs.org/blob/main/packages/ui-components/src/Icons/PartnerLogos/index.ts) which I mentioned I'm explicitly against.

Please remove such index.ts file and let people to manually import logos as needed.

We should probably also add https://github.com/nodejs/nodejs.org/blob/main/apps/site/next.config.mjs#L94 our ui-components package to this list.

ovflowd avatar Oct 29 '25 12:10 ovflowd

There's no way any bundler can know at build-time what logos will be used here or not: https://github.com/nodejs/nodejs.org/blob/a49f65c5676da4a2d0cb15feb0db196411179196/apps/site/util/partners/index.tsx#L1 / https://github.com/nodejs/nodejs.org/blob/a49f65c5676da4a2d0cb15feb0db196411179196/apps/site/util/partners/index.tsx#L14

This is just poor unoptimized code :sweat_smile:

ovflowd avatar Oct 29 '25 12:10 ovflowd