-
Notifications
You must be signed in to change notification settings - Fork 12.6k
chore(fuselage-ui-kit): Review build configuration #37708
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
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughRemoves Babel build tooling from the fuselage-ui-kit package and migrates to TypeScript compiler as the primary build tool. Consolidates separate CommonJS and ESM build configurations into a unified TypeScript build configuration. Simplifies ESLint setup and updates imports to type-only where appropriate. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Areas to verify:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/fuselage-ui-kit/tsconfig.build.json (1)
1-10: Tighten Storybook/test exclusion and confirmnoEmitsemanticsTwo small points to double‑check here:
- The Storybook config also uses a
../src/**/stories.tsxpattern. With the currentexclude: ["src/**/*.stories.tsx", …], files named exactlystories.tsx(without.stories.in the basename) will still be compiled intodist. If those are Storybook‑only entrypoints, consider also excluding them to keep the build clean:- "exclude": ["node_modules", "dist", "src/**/*.stories.tsx", ".storybook/**/*", "./src/**/*.spec.ts", "./src/**/*.spec.tsx"] + "exclude": [ + "node_modules", + "dist", + "src/**/*.stories.tsx", + "src/**/stories.tsx", + ".storybook/**/*", + "./src/**/*.spec.ts", + "./src/**/*.spec.tsx" + ]
- Since this extends
./tsconfig.json, please confirm the base config does not set"noEmit": true(or, if it does, explicitly override it here). Otherwisetsc -p tsconfig.build.jsonmight succeed but emit no files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
packages/fuselage-ui-kit/.babelrc.json(0 hunks)packages/fuselage-ui-kit/.eslintrc.json(1 hunks)packages/fuselage-ui-kit/.storybook/main.ts(1 hunks)packages/fuselage-ui-kit/.storybook/webpackAddon.ts(0 hunks)packages/fuselage-ui-kit/package.json(2 hunks)packages/fuselage-ui-kit/src/elements/IconButtonElement.tsx(1 hunks)packages/fuselage-ui-kit/src/elements/IconElement.tsx(1 hunks)packages/fuselage-ui-kit/tsconfig.build.json(1 hunks)packages/fuselage-ui-kit/tsconfig.cjs.json(0 hunks)packages/fuselage-ui-kit/tsconfig.esm.json(0 hunks)packages/fuselage-ui-kit/tsconfig.json(1 hunks)packages/livechat/package.json(1 hunks)
💤 Files with no reviewable changes (4)
- packages/fuselage-ui-kit/.storybook/webpackAddon.ts
- packages/fuselage-ui-kit/tsconfig.esm.json
- packages/fuselage-ui-kit/.babelrc.json
- packages/fuselage-ui-kit/tsconfig.cjs.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/fuselage-ui-kit/src/elements/IconElement.tsxpackages/fuselage-ui-kit/src/elements/IconButtonElement.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use `.spec.ts` extension for test files (e.g., `login.spec.ts`)
Applied to files:
packages/fuselage-ui-kit/tsconfig.build.json
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/fuselage-ui-kit/tsconfig.build.jsonpackages/fuselage-ui-kit/tsconfig.json
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
packages/fuselage-ui-kit/src/elements/IconElement.tsxpackages/fuselage-ui-kit/src/elements/IconButtonElement.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (7)
packages/fuselage-ui-kit/tsconfig.json (1)
3-6: Formatting adjustment aligns with build configuration consolidation.The rewrap and indentation adjustments here are purely stylistic with no semantic impact on the TypeScript configuration. The change supports the broader consolidation effort by standardizing this file's structure as legacy per-target configs are removed.
packages/fuselage-ui-kit/src/elements/IconElement.tsx (1)
2-2: LGTM! Correct type-only import conversions.Both
UiKitandBlockPropsare exclusively used in type positions, making these type-only import conversions accurate and aligned with TypeScript best practices. This reduces runtime bundle size and clarifies compile-time-only dependencies.Also applies to: 5-5
packages/fuselage-ui-kit/src/elements/IconButtonElement.tsx (1)
2-2: LGTM! Correct type-only import conversion.The
UiKitnamespace is exclusively used in type position (line 8), making this type-only import conversion accurate and consistent with TypeScript best practices.packages/livechat/package.json (1)
59-59: No issue identified: @babel/preset-react at ~7.27.1 aligns with the existing monorepo versioning strategy, where @babel/preset-typescript is already pinned to 7.27.1 in this same file. This is a consistent pattern across multiple packages (apps/meteor, ee/packages/presence, packages/core-services, etc.). Minor version differences between Babel 7.27.x and 7.28.x are compatible within the same major version.Likely an incorrect or invalid review comment.
packages/fuselage-ui-kit/.storybook/main.ts (1)
8-13: SWC compiler addon wiring looks consistentUsing
getAbsolutePath('@storybook/addon-webpack5-compiler-swc')in the addons array is consistent with how the other addons are resolved and matches an SWC‑based Storybook setup; no issues from this change.packages/fuselage-ui-kit/.eslintrc.json (1)
1-4: Simplified ESLint setup aligns with shared configsRelying on
@rocket.chat/eslint-config,@rocket.chat/eslint-config/react, andplugin:storybook/recommendedwith the current ignore patterns looks good and keeps this package consistent with the shared linting strategy.packages/fuselage-ui-kit/package.json (1)
20-21: Verify new single-build output and SWC addon compatibilityThe unified build setup looks coherent, but a couple of things are worth confirming:
Entry paths vs emitted files
main: "dist/index.js"andtypes: "dist/index.d.ts"now assume thattsc -p tsconfig.build.jsonemits those exact files.- Please run the build locally and confirm that:
dist/index.jsanddist/index.d.tsexist, and- there are no remaining consumers (internal or external) that still rely on the old
dist/cjs/index.js(or other CJS entrypoints).SWC compiler addon version
- The added
@storybook/addon-webpack5-compiler-swc@~3.0.0should be compatible with the Storybook 8.6.x stack declared here.- It’s worth quickly checking the addon’s docs or release notes against the Storybook version you’re using to ensure there are no known incompatibilities.
Also applies to: 28-29, 32-32, 64-64
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37708 +/- ##
===========================================
- Coverage 67.78% 67.77% -0.01%
===========================================
Files 3449 3449
Lines 113987 113987
Branches 20956 20956
===========================================
- Hits 77262 77258 -4
Misses 34610 34610
- Partials 2115 2119 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
a361af8 to
b9374b1
Compare
Proposed changes (including videos or screenshots)
It normalizes ESLint and Storybook configuration for
@rocket.chat/fuselage-ui-kit.Issue(s)
ARCH-1906
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.