fix: package lint issues #1773
fix: package lint issues #1773
Conversation
WalkthroughThese changes primarily focus on improving the build process, enforcing size limits for various packages, and updating code quality checks. Specific modifications include transitioning JavaScript files to use modern ESM and CJS formats, refining Rollup configurations for accurate builds, and adjusting size limits for more precise control over package sizes. Additionally, some import paths and utility scripts were updated for better maintainability and consistency. Changes
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (8)
nx.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.jsonpackages/analytics-js-cookies/package.jsonis excluded by!**/*.jsonpackages/analytics-js-plugins/package.jsonis excluded by!**/*.jsonpackages/analytics-js-service-worker/package.jsonis excluded by!**/*.jsonpackages/analytics-js/package.jsonis excluded by!**/*.jsonpackages/sanity-suite/package.jsonis excluded by!**/*.json
Files selected for processing (10)
- .github/workflows/security-code-quality-and-bundle-size-checks.yml (1 hunks)
- packages/analytics-js-cookies/.size-limit.mjs (1 hunks)
- packages/analytics-js-cookies/rollup.config.mjs (3 hunks)
- packages/analytics-js-plugins/.size-limit.mjs (1 hunks)
- packages/analytics-js-plugins/rollup.config.mjs (3 hunks)
- packages/analytics-js-service-worker/.size-limit.mjs (1 hunks)
- packages/analytics-js-service-worker/rollup.config.mjs (5 hunks)
- packages/analytics-js/.size-limit.mjs (1 hunks)
- packages/analytics-js/rollup.config.mjs (3 hunks)
- packages/analytics-v1.1/tests/utils/navigator.test.js (1 hunks)
Files skipped from review due to trivial changes (6)
- .github/workflows/security-code-quality-and-bundle-size-checks.yml
- packages/analytics-js-cookies/.size-limit.mjs
- packages/analytics-js-plugins/.size-limit.mjs
- packages/analytics-js-service-worker/.size-limit.mjs
- packages/analytics-js/.size-limit.mjs
- packages/analytics-v1.1/tests/utils/navigator.test.js
Additional context used
Learnings (3)
packages/analytics-js-service-worker/rollup.config.mjs (1)
Learnt from: saikumarrs PR: rudderlabs/rudder-sdk-js#1759 File: packages/analytics-js-cookies/rollup.config.mjs:1-20 Timestamp: 2024-06-25T08:25:30.628Z Learning: User `saikumarrs` prefers to temporarily accept missing dependencies in `package.json` for `rollup.config.mjs` in the context of PR 1759 without immediate correction.packages/analytics-js-cookies/rollup.config.mjs (1)
Learnt from: saikumarrs PR: rudderlabs/rudder-sdk-js#1759 File: packages/analytics-js-cookies/rollup.config.mjs:1-20 Timestamp: 2024-06-25T08:25:30.628Z Learning: User `saikumarrs` prefers to temporarily accept missing dependencies in `package.json` for `rollup.config.mjs` in the context of PR 1759 without immediate correction.packages/analytics-js/rollup.config.mjs (1)
Learnt from: saikumarrs PR: rudderlabs/rudder-sdk-js#1759 File: packages/analytics-js-cookies/rollup.config.mjs:1-20 Timestamp: 2024-06-25T08:25:30.628Z Learning: User `saikumarrs` prefers to temporarily accept missing dependencies in `package.json` for `rollup.config.mjs` in the context of PR 1759 without immediate correction.
Biome
packages/analytics-js/rollup.config.mjs
[error] 70-72: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Additional comments not posted (9)
packages/analytics-js-service-worker/rollup.config.mjs (3)
16-17: New imports fordelandaliasplugins.The addition of
delandaliasplugins is appropriate for managing file deletions and aliasing paths, respectively. Ensure that these plugins are listed inpackage.json.Verification successful
New imports for
delandaliasplugins.The addition of
delandaliasplugins is appropriate for managing file deletions and aliasing paths, respectively. These plugins are confirmed to be listed inpackage.json.
rollup-plugin-delete@rollup/plugin-aliasScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of `del` and `alias` plugins in package.json. # Test: Search for the plugins in the dependencies. Expect: Both plugins to be listed. rg --files-with-matches 'rollup-plugin-delete|@rollup/plugin-alias' package.jsonLength of output: 95
Line range hint
108-126: LGTM!The new entry file names (
index.mjs,index.cjs,index.js) align with modern module formats.Ensure that the output directories and formats are correctly configured.
Verification successful
LGTM!
The output directories and formats are correctly configured:
outDiris set todist/npm.- Formats include
esm,cjs,umd, andes.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output configurations. # Test: Search for the output directories and formats. Expect: Correct configurations. rg --files-with-matches 'outDir|format' packages/analytics-js-service-worker/rollup.config.mjsLength of output: 151
Script:
#!/bin/bash # Description: Extract lines containing 'outDir' and 'format' for detailed inspection. # Extract and display lines with 'outDir' and 'format' in the specified file. rg 'outDir|format' packages/analytics-js-service-worker/rollup.config.mjsLength of output: 431
164-172: LGTM!The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats.Ensure that the output paths are correctly configured.
Verification successful
LGTM!
The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats and are correctly configured.
- Correct configurations found in
packages/analytics-js-service-worker/rollup.config.mjsaround lines 164-172.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output paths. # Test: Search for the output paths. Expect: Correct configurations. rg --files-with-matches 'output|file' packages/analytics-js-service-worker/rollup.config.mjsLength of output: 149
Script:
#!/bin/bash # Description: Extract and display the relevant portions of the rollup.config.mjs file where `file` and `output` are configured. # Extract lines containing 'output' or 'file' and display the surrounding context for better understanding. rg 'output|file' -C 3 packages/analytics-js-service-worker/rollup.config.mjsLength of output: 1327
packages/analytics-js-cookies/rollup.config.mjs (2)
Line range hint
107-117: LGTM!The new entry file names (
index.mjs,index.cjs) align with modern module formats.Ensure that the output directories and formats are correctly configured.
Verification successful
LGTM!
The output directories and formats are correctly configured. The new entry file names (
index.mjs,index.cjs) align with modern module formats, and the output paths and formats are appropriately set.
esmformat is directed todist/npm/esm/cjsformat is directed todist/npm/cjs/umdformat is directed todist/npm/umd/- Declaration files are using the
esformat.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output configurations. # Test: Search for the output directories and formats. Expect: Correct configurations. rg --files-with-matches 'outDir|format' packages/analytics-js-cookies/rollup.config.mjsLength of output: 137
Script:
#!/bin/bash # Extract lines containing 'outDir' and 'format' to verify their configurations rg 'outDir|format' packages/analytics-js-cookies/rollup.config.mjsLength of output: 438
165-173: LGTM!The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats.Ensure that the output paths are correctly configured.
Verification successful
LGTM!
The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats and are correctly configured in therollup.config.mjsfile.
- Lines 165-173 in
packages/analytics-js-cookies/rollup.config.mjsconfirm the correct configuration.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output paths. # Test: Search for the output paths. Expect: Correct configurations. rg --files-with-matches 'output|file' packages/analytics-js-cookies/rollup.config.mjsLength of output: 135
Script:
#!/bin/bash # Extract and display lines 160-180 from the rollup.config.mjs file to verify the output paths configuration. sed -n '160,180p' packages/analytics-js-cookies/rollup.config.mjsLength of output: 411
packages/analytics-js-plugins/rollup.config.mjs (2)
Line range hint
155-165: LGTM!The new entry file names (
index.mjs,index.cjs) align with modern module formats.Ensure that the output directories and formats are correctly configured.
Verification successful
Output configurations verified
The output directories and formats are correctly configured:
ESM format is set for
outDirNpm + '/esm/'andoutDirCDN.CJS format is set for
outDirNpm + '/cjs'.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output configurations. # Test: Search for the output directories and formats. Expect: Correct configurations. rg --files-with-matches 'outDir|format' packages/analytics-js-plugins/rollup.config.mjsLength of output: 137
Script:
#!/bin/bash # Description: Extract configurations for output directories and formats. # Extract lines containing 'outDir' and 'format' to verify their correctness. rg 'outDir|format' packages/analytics-js-plugins/rollup.config.mjsLength of output: 631
241-250: LGTM!The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats.Ensure that the output paths are correctly configured.
Verification successful
LGTM!
The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats.
packages/analytics-js-plugins/rollup.config.mjslines 241-250:
${outDirNpmRoot}/index.d.mts${outDirNpmRoot}/index.d.ctsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output paths. # Test: Search for the output paths. Expect: Correct configurations. rg --files-with-matches 'output|file' packages/analytics-js-plugins/rollup.config.mjsLength of output: 135
Script:
#!/bin/bash # Extract lines 241-250 from the rollup.config.mjs file to verify output paths sed -n '241,250p' packages/analytics-js-plugins/rollup.config.mjsLength of output: 275
packages/analytics-js/rollup.config.mjs (2)
Line range hint
289-299: LGTM!The new entry file names (
index.mjs,index.cjs) align with modern module formats.Ensure that the output directories and formats are correctly configured.
Verification successful
Output directories and formats are correctly configured.
The
outDirandformatsettings for the new entry file names (index.mjsandindex.cjs) align with the expected configurations for modern module formats.
index.mjsis configured withdir: outDirNpm + '/esm/'andformat: 'esm'.index.cjsis configured withdir: outDirNpm + '/cjs'andformat: 'cjs'.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output configurations. # Test: Search for the output directories and formats. Expect: Correct configurations. rg --files-with-matches 'outDir|format' packages/analytics-js/rollup.config.mjsLength of output: 121
Script:
#!/bin/bash # Extract and display lines containing 'outDir' and 'format' configurations in the rollup.config.mjs file rg 'outDir|format' packages/analytics-js/rollup.config.mjs -A 3 -B 3Length of output: 2667
382-391: LGTM!The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats.Ensure that the output paths are correctly configured.
Verification successful
LGTM!
The new output paths for
.d.tsfiles (index.d.mts,index.d.cts) align with modern TypeScript module formats and are correctly configured.
packages/analytics-js/rollup.config.mjslines 382-391Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of output paths. # Test: Search for the output paths. Expect: Correct configurations. rg --files-with-matches 'output|file' packages/analytics-js/rollup.config.mjsLength of output: 119
Script:
#!/bin/bash # Description: Extract the specific configurations for output paths in rollup.config.mjs # Use ast-grep to find the output configurations in the rollup config file ast-grep --lang javascript --pattern $'output: [ { file: $_, format: $_, }, { file: $_, format: $_, } ]' packages/analytics-js/rollup.config.mjsLength of output: 169
Script:
#!/bin/bash # Description: Extract the specific configurations for output paths in rollup.config.mjs # Use rg to find the output configurations in the rollup config file rg 'output: \[' packages/analytics-js/rollup.config.mjs -A 10Length of output: 279
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1773 +/- ##
========================================
Coverage 56.26% 56.26%
========================================
Files 467 467
Lines 15912 15912
Branches 3188 3177 -11
========================================
Hits 8953 8953
- Misses 5717 5732 +15
+ Partials 1242 1227 -15 ☔ View full report in Codecov by Sentry. |
size-limit report 📦
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (10)
package.jsonis excluded by!**/*.jsonpackages/analytics-js-common/package.jsonis excluded by!**/*.jsonpackages/analytics-js-cookies/package.jsonis excluded by!**/*.jsonpackages/analytics-js-integrations/package.jsonis excluded by!**/*.jsonpackages/analytics-js-plugins/package.jsonis excluded by!**/*.jsonpackages/analytics-js-service-worker/package.jsonis excluded by!**/*.jsonpackages/analytics-js/package.jsonis excluded by!**/*.jsonpackages/analytics-v1.1/package.jsonis excluded by!**/*.jsonpackages/loading-scripts/package.jsonis excluded by!**/*.jsonpackages/sanity-suite/package.jsonis excluded by!**/*.json
Files selected for processing (3)
- packages/analytics-js-integrations/.size-limit.js (1 hunks)
- packages/analytics-js-integrations/scripts/integrationBuildScript.js (1 hunks)
- packages/analytics-js-service-worker/.size-limit.mjs (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/analytics-js-integrations/.size-limit.js
Files skipped from review as they are similar to previous changes (1)
- packages/analytics-js-service-worker/.size-limit.mjs
Additional comments not posted (1)
packages/analytics-js-integrations/scripts/integrationBuildScript.js (1)
5-5: Verify the alias configuration.Ensure that the alias
@rudderstack/analytics-js-commonis correctly configured in the project's build system (e.g.,tsconfig.json,webpack.config.js, etc.).Verification successful
Alias configuration verified.
The alias
@rudderstack/analytics-js-commonis correctly configured in the project's build system, as evidenced by its presence intsconfig.jsonandrollup.config.mjs. The extensive usage across the codebase further confirms its proper setup.
tsconfig.json:"@rudderstack/analytics-js-common/*": ["./packages/analytics-js-common/src/*"]rollup.config.mjs:find: '@rudderstack/analytics-js-common'Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the alias configuration for @rudderstack/analytics-js-common. # Test: Search for the alias configuration. Expect: Alias should be defined in the build system configuration files. rg '@rudderstack/analytics-js-common'Length of output: 141171
saikumarrs
requested review from
ItsSudip,
shrouti1507,
sandeepdsvs,
koladilip and
a team
as code owners
|
PR Description
I've addressed all the linting issues in the packages.
Fixes:
#1757
Linear task (optional)
https://linear.app/rudderstack/issue/SDK-2052/investigate-github-issue
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
Summary by CodeRabbit
Chores
Refactor