Skip to content

Build esm and cjs versions of nimbus-icons to insure compatibility in non-esm environments (eg jest) #91

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

Closed
wants to merge 11 commits into from

Conversation

ByronDWall
Copy link
Contributor

@ByronDWall ByronDWall commented Apr 30, 2025

This pull request includes updates to the @commercetools/nimbus-icons package, focusing on restructuring its build process, modifying the package configuration, and adding new material icon components. The most significant changes include updates to the package.json file for better module support, adjustments to the .gitignore file, and the addition of multiple new material icon components.

Build and configuration updates:

  • Updated packages/nimbus-icons/package.json to include separate CommonJS (cjs) and ES Module (esm) builds, added sideEffects for tree-shaking, and defined exports for better module resolution. Updated the version to 0.0.4.
  • Modified build scripts in package.json to use separate TypeScript configurations (tsconfig.esm.json and tsconfig.cjs.json) for esm and cjs builds.
  • Updated .gitignore to ignore all files in src/material-icons/ instead of the directory itself.

New material icon components:

  • Added 20+ new material icon components, such as 10k.tsx, 10mp.tsx, 11mp.tsx, and 1k.tsx, each implementing a React component with forwardRef for better integration and accessibility. [1] [2] [3] [4] etc.)

Copy link

vercel bot commented Apr 30, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nimbus-documentation ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 7, 2025 2:23pm
nimbus-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 7, 2025 2:23pm

@ByronDWall ByronDWall requested review from a team, stephsprinkle, jaikamat, ddouglasz, valoriecarli, tylermorrisford and misama-ct and removed request for a team May 1, 2025 15:16
@ByronDWall ByronDWall self-assigned this May 1, 2025
@@ -1,9 +1,22 @@
{
"name": "@commercetools/nimbus-icons",
"version": "0.0.2",
"version": "0.0.3-rc1",
"main": "dist/index.js",
"module": "./dist/index.js",
"types": "./dist/index.d.ts",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is anything speaking against adding "sideEffects": false to the package.json? I tried it on my blank-app branch cause the empty (!) app was huge. And it shoved off roughly a Megabyte:

Suggested change
"types": "./dist/index.d.ts",
"types": "./dist/index.d.ts",
"sideEffects": false,

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@misama-ct misama-ct left a comment

Choose a reason for hiding this comment

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

LGTM 👍

…ignore in nimbus-icons to ignore everything in /material-icons but keep the folder so that its there for vercel hopefully
Copy link
Collaborator

@misama-ct misama-ct left a comment

Choose a reason for hiding this comment

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

As mentioned, I got this build-process working without the need to commit the 2k+ icons. Feel free to hijack the branch or incorporate the changes into this one.

@@ -20,7 +33,7 @@
},
"scripts": {
"build-icons": "pnpm svgr --filename-case kebab --typescript --jsx-runtime automatic --out-dir src/material-icons node_modules/@material-design-icons/svg/outlined",
"build": "pnpm run build-icons && tsc",
"dev": "tsc"
"build": "pnpm run build-icons && tsc --project tsconfig.esm.json & tsc --project tsconfig.cjs.json",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I managed to get the build-process to work without committing the icons. The only "significant" change I did was running the two tsc tasks in sequence instead of in parallel/in the background:

Suggested change
"build": "pnpm run build-icons && tsc --project tsconfig.esm.json & tsc --project tsconfig.cjs.json",
"build": "pnpm run build-icons && tsc --project tsconfig.esm.json && tsc --project tsconfig.cjs.json",

Copy link

@orca-security-eu orca-security-eu bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Failed Failed Vulnerabilities high 1   medium 0   low 0   info 0 View in Orca
☢️ The following Vulnerabilities (CVEs) have been detected
PACKAGE FILE CVE ID INSTALLED VERSION FIXED VERSION
high trim-newlines ./pnpm-lock.yaml CVE-2021-33623 1.0.0 3.0.1, 4.0.1 View in code

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