chore: clean up build configurations #2060
Conversation
📝 WalkthroughWalkthroughThis pull request removes references to Bugsnag error tracking and deprecated dynamic bundle configurations from several files. The changes delete the Changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## hotfix/unsuppress-npm-errors #2060 +/- ##
=============================================================
Coverage 61.41% 61.41%
=============================================================
Files 478 478
Lines 16404 16404
Branches 3232 3245 +13
=============================================================
Hits 10075 10075
+ Misses 5122 5105 -17
- Partials 1207 1224 +17 ☔ View full report in Codecov by Sentry. |
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/analytics-js/rollup.config.mjs (1)
166-276: Consider refactoring the plugin configuration.The plugin configuration section is quite lengthy and could benefit from being split into smaller, more focused functions. This would improve maintainability and make the code easier to understand.
Consider extracting plugin configurations into separate functions:
- return { - watch: { - include: ['src/**', ...additionalWatchPaths], - }, - external: [/rudderAnalyticsRemotePlugins\/.*/, ...Object.keys(pkg.peerDependencies || {})], - onwarn(warning, warn) { - if (warning.code === 'THIS_IS_UNDEFINED') { - return; - } - warn(warning); - }, - plugins: [ - replace({...}), - resolve({...}), - // ... more plugins - ], - }; + const getWatchConfig = () => ({ + include: ['src/**', ...additionalWatchPaths], + }); + + const getExternalConfig = () => [ + /rudderAnalyticsRemotePlugins\/.*/, + ...Object.keys(pkg.peerDependencies || {}), + ]; + + const getWarningHandler = () => (warning, warn) => { + if (warning.code === 'THIS_IS_UNDEFINED') { + return; + } + warn(warning); + }; + + const getPluginsConfig = () => [ + replace({...}), + resolve({...}), + // ... more plugins + ]; + + return { + watch: getWatchConfig(), + external: getExternalConfig(), + onwarn: getWarningHandler(), + plugins: getPluginsConfig(), + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.github/workflows/deploy-sanity-suite.ymlis excluded by!**/*.yml
📒 Files selected for processing (8)
.env.example(0 hunks)jest/jest.setup-dom.js(0 hunks)packages/analytics-js-plugins/rollup.config.mjs(0 hunks)packages/analytics-js-service-worker/.env.example(0 hunks)packages/analytics-js/.env.example(0 hunks)packages/analytics-js/rollup.config.mjs(3 hunks)packages/loading-scripts/rollup.config.mjs(0 hunks)types/global.d.ts(0 hunks)
💤 Files with no reviewable changes (7)
- .env.example
- jest/jest.setup-dom.js
- types/global.d.ts
- packages/analytics-js-service-worker/.env.example
- packages/analytics-js-plugins/rollup.config.mjs
- packages/analytics-js/.env.example
- packages/loading-scripts/rollup.config.mjs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Bundle size checks
🔇 Additional comments (3)
packages/analytics-js/rollup.config.mjs (3)
31-31: LGTM! Improved plugin bundling logic.The new
bundleAllPluginsconstant simplifies the logic for determining when to bundle all plugins, making it more maintainable and easier to understand.Also applies to: 183-183
43-43: LGTM! Simplified CDN path logic.The
outDirCDNRootassignment has been simplified by removing the conditional logic, which aligns with the PR's objective of cleaning up build configurations.
189-189:❓ Verification inconclusive
Verify Bugsnag release stage configuration.
While Bugsnag API key and URL requirements are being removed, the
__RS_BUGSNAG_RELEASE_STAGE__replacement is still present. This might need to be removed for consistency.
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining Bugsnag-related configurations # Search for Bugsnag-related configurations in the codebase rg -i "bugsnag" --type js --type ts --type jsonLength of output: 47
I've noticed that the initial search didn't return any Bugsnag references—likely because it excluded
.mjsfiles (where our reference lives). To double-check, please verify the existence and usage of this configuration in the Rollup config by running a search on the file itself:Once you confirm that
__RS_BUGSNAG_RELEASE_STAGE__appears only in this file and isn’t used elsewhere in the codebase, please verify if it’s still needed. Given that other Bugsnag configurations (like the API key and URL) are already removed, it might be best for consistency to also remove this release stage configuration—provided it doesn’t affect any legacy behavior.Action: Confirm whether
__RS_BUGSNAG_RELEASE_STAGE__is required. If not, remove it frompackages/analytics-js/rollup.config.mjs.
🏁 Script executed:
#!/bin/bash rg -i "BUGSNAG_RELEASE_STAGE" packages/analytics-js/rollup.config.mjsLength of output: 149
Action: Verify and assess the necessity of the Bugsnag release stage configuration
It appears the initial search didn’t locate the expected file because the filename (or extension) might differ from what was assumed (i.e.
rollup.config.mjs). To clarify the situation, please run this script to search for any occurrences of__RS_BUGSNAG_RELEASE_STAGE__in all potential rollup configuration files under thepackages/analytics-jsdirectory:Once you review the output:
- If the
__RS_BUGSNAG_RELEASE_STAGE__configuration is only found in legacy rollup settings and isn’t required given that other Bugsnag configurations (e.g., API key and URL) have been removed, please remove it for consistency.- If it turns out that this configuration is still actively used or required for legacy support, please document its purpose accordingly.
🏁 Script executed:
#!/bin/bash # Locate any rollup configuration files (with .mjs or .js extensions) under packages/analytics-js files=$(fd -e mjs -e js "rollup.config" packages/analytics-js) if [ -z "$files" ]; then echo "No rollup configuration files found in packages/analytics-js." else for file in $files; do echo "Searching in: $file" rg -n "__RS_BUGSNAG_RELEASE_STAGE__" "$file" done fiLength of output: 321
Action: Confirm the necessity of the Bugsnag release stage configuration
Our searches indicate that there is no evidence of a rollup configuration file in the expected location (i.e. under “packages/analytics-js”). In particular, attempts to locate occurrences of
__RS_BUGSNAG_RELEASE_STAGE__returned no results, which may imply that this configuration is either no longer in use or the file has been relocated.
- Please manually verify:
- Whether the configuration snippet (
__RS_BUGSNAG_RELEASE_STAGE__: process.env.BUGSNAG_RELEASE_STAGE || 'production') is present in any rollup configuration file.- If it’s not used anywhere else in the codebase, confirm that it can safely be removed for consistency with the removal of other Bugsnag settings.
Once you confirm the configuration isn’t required, please proceed to remove it.
size-limit report 📦
|
PR Description
I've fixed several issues and cleaned up stale items in the packages` build configurations:
__SDK_BUNDLE_FILENAME__from appropriate packages.The clean up helps us in maintaining the build configurations better now that the clutter has been cleared.
Linear task (optional)
https://linear.app/rudderstack/issue/SDK-3012/clean-up-build-configurations
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
Summary by CodeRabbit
Chores
Refactor
These updates aim to reduce clutter in configuration files and improve the overall development and build process, ensuring a smoother setup and deployment experience for users.