3.0.1 #73
3.0.1 #73
Conversation
Apply Sweep Rules to your PR?
|
1 similar comment
Apply Sweep Rules to your PR?
|
📝 WalkthroughWalkthroughConvert service worker registration to use globalThis consistently, add a production guard to abort registration when PUBLIC_URL is unset, and refactor lifecycle handling and reload/origin checks within the service worker registration flow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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
Outside diff range, codebase verification and nitpick comments (1)
apps/react-vite/src/App.css (1)
1-42: LGTM, but consider customizing the styles.The CSS styles in the file look good. They are standard styles that are commonly used in React applications and do not contain any obvious issues.
However, the styles seem to be based on a template or boilerplate. Consider customizing the styles to match the specific design and requirements of your application.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
apps/react-vite/public/vite.svgis excluded by!**/*.svgapps/react-vite/src/assets/react.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (22)
- README.md (1 hunks)
- apps/nextjs-beta/package.json (2 hunks)
- apps/nextjs/package.json (1 hunks)
- apps/nextjs/pages/index.js (3 hunks)
- apps/react-vite/.eslintrc.cjs (1 hunks)
- apps/react-vite/.gitignore (1 hunks)
- apps/react-vite/README.md (1 hunks)
- apps/react-vite/index.html (1 hunks)
- apps/react-vite/package.json (1 hunks)
- apps/react-vite/src/App.css (1 hunks)
- apps/react-vite/src/App.tsx (1 hunks)
- apps/react-vite/src/index.css (1 hunks)
- apps/react-vite/src/main.tsx (1 hunks)
- apps/react-vite/src/vite-env.d.ts (1 hunks)
- apps/react-vite/tsconfig.json (1 hunks)
- apps/react-vite/tsconfig.node.json (1 hunks)
- apps/react-vite/vite.config.ts (1 hunks)
- apps/react/package.json (1 hunks)
- components/react/README.md (1 hunks)
- components/react/lib/Particles.tsx (1 hunks)
- components/react/lib/index.ts (1 hunks)
- components/react/package.json (1 hunks)
Files skipped from review due to trivial changes (11)
- README.md
- apps/nextjs-beta/package.json
- apps/react-vite/.gitignore
- apps/react-vite/index.html
- apps/react-vite/src/index.css
- apps/react-vite/src/main.tsx
- apps/react-vite/src/vite-env.d.ts
- apps/react-vite/vite.config.ts
- apps/react/package.json
- components/react/README.md
- components/react/package.json
Additional comments not posted (13)
apps/react-vite/tsconfig.node.json (1)
1-10: LGTM!The
tsconfig.node.jsonfile is correctly configured for a Node.js environment used by Vite.components/react/lib/index.ts (1)
9-9: LGTM!The dynamic import of the
tsParticlesmodule is a good optimization that allows for the module to be loaded only when theinitParticlesEnginefunction is invoked, potentially improving performance by reducing the initial load time of the module.apps/react-vite/.eslintrc.cjs (1)
1-18: LGTM!The
.eslintrc.cjsfile is correctly configured for a React application using Vite.
- It extends recommended configurations and enables additional plugins for TypeScript and React.
- The
react-refresh/only-export-componentsrule is correctly configured to allow constant exports.- The
ignorePatternsproperty is correctly configured to ignore thedistdirectory and the.eslintrc.cjsfile itself.apps/nextjs/package.json (1)
13-19: LGTM!The changes to the
@tsparticlesdependencies look good. The removal of@tsparticles/preset-big-circles, the addition of@tsparticles/configsandtsparticles, and the version upgrade of@tsparticles/engineare consistent with the PR summary.apps/react-vite/tsconfig.json (1)
1-25: LGTM!The
tsconfig.jsonfile looks good. The compiler options are appropriately configured for a modern React application using Vite. The strict type checking options will help catch potential issues. Theincludeandreferencesoptions are also set correctly.components/react/lib/Particles.tsx (1)
11-21: LGTM!The changes to the
useEffecthook introduce a more efficient loading strategy for thetsParticlesmodule. By dynamically importing the module within an async function, the initial rendering of the component is not blocked. The overall functionality remains intact, and theparticlesLoadedcallback is still invoked with the loaded container.The code changes are approved.
apps/react-vite/package.json (1)
1-31: LGTM!The
package.jsonfile is a standard configuration file for a React application using Vite. The dependencies and devDependencies are necessary for the application to function properly.The code changes are approved.
apps/react-vite/README.md (1)
1-30: LGTM!The
README.mdfile provides helpful information about the React + TypeScript + Vite template and instructions for expanding the ESLint configuration.The code changes are approved.
apps/react-vite/src/App.tsx (1)
1-72: LGTM!The code changes in this new file are approved. The React component is well-structured and follows best practices. It correctly initializes the particles engine, loads the full package, and renders the
Particlescomponent with appropriate options. The component also properly manages the particles initialization state using theuseStateanduseEffecthooks.apps/nextjs/pages/index.js (4)
4-4: LGTM!The import statement for
configsfrom@tsparticles/configsis correct and follows the proper syntax.
5-5: LGTM!The import statement for
loadFullfromtsparticlesis correct and follows the proper syntax.
13-13: LGTM!The change from
loadBigCirclesPresettoloadFullis appropriate as it suggests a shift from a specific preset to a more comprehensive loading function. Theawaitkeyword is used correctly to wait for theloadFullfunction to complete before proceeding.
92-92: LGTM!The change from
options={options}tooptions={configs.basic}is a good improvement as it suggests that the particle configuration is now being sourced from a centralized configuration file. This change enhances maintainability and flexibility by allowing for easier updates to particle settings through the configuration file.
There was a problem hiding this comment.
Pull request overview
This PR updates the repository to version 3.0.1, focusing on dependency upgrades, performance improvements through dynamic imports, and adding a new Vite-based React application template. The main changes include updating @tsparticles packages from 3.0.2 to 3.2.2, implementing dynamic loading for the tsParticles engine in React components to improve code splitting, and replacing window references with globalThis in the service worker for better cross-environment compatibility.
Key Changes:
- Updated @tsparticles dependencies from 3.0.2/3.0.0 to 3.2.2 across all packages
- Implemented dynamic imports in React components to enable lazy loading of the tsParticles engine
- Replaced
windowwithglobalThisin serviceWorker.ts for improved cross-environment compatibility - Added new react-vite application with TypeScript and Vite configuration
- Removed Slack communication channel from README
Reviewed changes
Copilot reviewed 22 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| templates/react-ts/template/src/serviceWorker.ts | Updated to use globalThis instead of window for better compatibility |
| components/react/lib/index.ts | Added dynamic import for tsParticles engine |
| components/react/lib/Particles.tsx | Implemented async dynamic import for tsParticles engine with void IIFE |
| components/react/package.json | Updated @tsparticles/engine peer/dev dependencies to 3.2.2 |
| components/react/README.md | Removed Slack badge from communication channels |
| apps/react/package.json | Updated @tsparticles dependencies to 3.2.2 |
| apps/react-vite/* | Added new Vite-based React application with complete setup |
| pnpm-lock.yaml | Updated lockfile with new dependency versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @templates/react-ts/template/src/serviceWorker.ts:
- Around line 13-21: The code uses globalThis in the service worker
detection/registration logic (e.g., the isLocalhost constant and the
registration flow) which is incompatible with the project's ES5 target; replace
all uses of globalThis with window (for example change
globalThis.location.hostname to window.location.hostname and any other
globalThis references around the service worker
registration/registrationOnSuccess logic) so the browser-only service worker
code uses window and restores ES5 compatibility.
🧹 Nitpick comments (1)
templates/react-ts/template/src/serviceWorker.ts (1)
42-42: Consider semantic clarity:windowvsglobalThisfor browser-specific APIs.While
globalThis.addEventListener('load', ...)is functionally correct in browser contexts, theloadevent is specific to thewindowobject. Service worker registration inherently requires a browser window context, so usingwindow.addEventListenerwould be more semantically explicit about the execution environment.This is not a functional issue, but it may affect code readability and intent clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/react-ts/template/src/serviceWorker.ts
⏰ 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: Upload results
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
templates/react-ts/template/src/serviceWorker.ts (2)
31-40: Changes look correct for URL construction and origin validation.The migration to
globalThis.location.href(line 33) andglobalThis.location.origin(line 35) is functionally correct. These changes maintain the expected behavior for validating that the service worker and page are served from the same origin.
124-124: Migration toglobalThis.location.reload()is consistent.The change from
window.location.reload()toglobalThis.location.reload()is functionally correct and consistent with the other location API updates throughout the file.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @templates/react-ts/template/src/serviceWorker.ts:
- Around line 29-31: The guard currently requires process.env.PUBLIC_URL to be
truthy which skips service worker registration when PUBLIC_URL is unset/empty;
change the check to normalize PUBLIC_URL first (e.g., compute a publicUrlEnv
variable that defaults to '/' or to new URL('.',
window.location.href).toString() when process.env.PUBLIC_URL is falsy) and then
use that publicUrlEnv in the if condition and later code paths that reference
PUBLIC_URL so registrations still occur for the root/public default while
preserving explicit PUBLIC_URL behavior.
- Around line 13-21: The isLocalhost check uses globalThis.location.hostname (an
ES2020 feature) which breaks when tsconfig.target is ES5; replace references to
globalThis.location.hostname with window.location.hostname in the isLocalhost
computation (or, alternatively, add a globalThis polyfill via
core-js/@babel/preset-env and ensure it's imported before this module) so the
hostname checks (including the IPv6 and 127.* regex match) run in older
browsers.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/react-ts/template/src/serviceWorker.ts
⏰ 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). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
templates/react-ts/template/src/serviceWorker.ts (4)
33-43: LGTM: URL construction and origin validation.The URL construction using
globalThis.location.hrefas the base and the origin validation to prevent CDN issues are correctly implemented.
45-64: LGTM: Event listener and registration flow.The migration to
globalThis.addEventListeneris consistent with other changes, and the localhost vs. production registration logic remains sound.
67-109: LGTM: Service worker lifecycle handling.The refactored indentation improves readability while preserving the correct lifecycle logic for update and success callbacks.
111-151: LGTM: Service worker validation and unregistration.The migration to
globalThis.location.reload()at line 126 is consistent with the rest of the file. The validation logic and unregistration flow remain correct.
| const isLocalhost = Boolean( | ||
| window.location.hostname === 'localhost' || | ||
| globalThis.location.hostname === 'localhost' || | ||
| // [::1] is the IPv6 localhost address. | ||
| window.location.hostname === '[::1]' || | ||
| globalThis.location.hostname === '[::1]' || | ||
| // 127.0.0.0/8 are considered localhost for IPv4. | ||
| window.location.hostname.match( | ||
| globalThis.location.hostname.match( | ||
| /^127(?:\.(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}$/ | ||
| ) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
globalThis browser support ES2020 compatibility older browsers IE11 Safari
💡 Result:
Short answer
- globalThis is part of modern ECMAScript and is supported in current browsers but not in IE11. Use a polyfill or transpilation for older browsers. [1][2][3][4]
Details (concise)
- Support: Chrome 71+, Firefox 65+, Safari 12.1+/iOS Safari 12.2+, Edge (Chromium) 79+. Internet Explorer (including IE11) has no support. [1][2][3]
- Practical options for IE11 / older Safari:
- Add a small runtime polyfill/fallback (or use core‑js). Mathias Bynens’ polyfill is a standard reference. [4]
- Or use @babel/preset-env / core-js via your build to provide globalThis and other ES2020 features to older browsers. [1][4]
If you want, I can paste a minimal reliable polyfill or show how to configure Babel/core-js to include globalThis.
🏁 Script executed:
# Check for project configuration files that define browser targets
git ls-files | grep -E "(tsconfig|babel|browserslist|package\.json)" | head -20Repository: tsparticles/react
Length of output: 477
🏁 Script executed:
# Read tsconfig.json to check compilation target
find . -name "tsconfig.json" -type f | head -5Repository: tsparticles/react
Length of output: 202
🏁 Script executed:
# Check the target in the relevant template's tsconfig.json
cat -n ./templates/react-ts/template/tsconfig.jsonRepository: tsparticles/react
Length of output: 761
🏁 Script executed:
# Check for babel configuration in the template directory
find ./templates/react-ts/template -name ".babelrc*" -o -name "babel.config.*" | head -5Repository: tsparticles/react
Length of output: 43
🏁 Script executed:
# Check the package.json in the template to see browser targets and dependencies
cat -n ./templates/react-ts/template/package.jsonRepository: tsparticles/react
Length of output: 135
🏁 Script executed:
# Check the template directory structure
ls -la ./templates/react-ts/Repository: tsparticles/react
Length of output: 599
🏁 Script executed:
# Look for package.json in the template root
find ./templates/react-ts -maxdepth 2 -name "package.json" -type fRepository: tsparticles/react
Length of output: 94
🏁 Script executed:
# Check the actual serviceWorker.ts file to see context
head -30 ./templates/react-ts/template/src/serviceWorker.tsRepository: tsparticles/react
Length of output: 1300
🏁 Script executed:
# Check for browserslist configuration
find ./templates/react-ts -name ".browserslistrc" -o -name "browserslist" | head -5Repository: tsparticles/react
Length of output: 43
🏁 Script executed:
# Check the main package.json in the template directory for build scripts and dependencies
cat -n ./templates/react-ts/package.jsonRepository: tsparticles/react
Length of output: 3325
🏁 Script executed:
# Check if there's a build config or further details in template.json
cat -n ./templates/react-ts/template.jsonRepository: tsparticles/react
Length of output: 710
Migrate from globalThis to window or add a polyfill for ES5 compatibility.
The project's TypeScript configuration targets ES5 ("target": "es5" in tsconfig.json), but the code uses globalThis, which is an ES2020 feature not supported in IE11 or older Safari versions (pre-12.1). With an ES5 target, TypeScript will not transpile globalThis, causing runtime errors in older browsers.
Either revert to window.location (available in all ES5+ environments) or add a polyfill using core-js / @babel/preset-env to support globalThis across older browsers.
🤖 Prompt for AI Agents
In @templates/react-ts/template/src/serviceWorker.ts around lines 13 - 21, The
isLocalhost check uses globalThis.location.hostname (an ES2020 feature) which
breaks when tsconfig.target is ES5; replace references to
globalThis.location.hostname with window.location.hostname in the isLocalhost
computation (or, alternatively, add a globalThis polyfill via
core-js/@babel/preset-env and ensure it's imported before this module) so the
hostname checks (including the IPv6 and 127.* regex match) run in older
browsers.
| if (!(process.env.NODE_ENV === 'production' && 'serviceWorker' in navigator && process.env.PUBLIC_URL)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Breaking change: PUBLIC_URL is now required for service worker registration.
The guard condition now requires process.env.PUBLIC_URL to be truthy. If PUBLIC_URL is undefined, empty string, or not set, service worker registration will be silently skipped in production.
This could break existing deployments that don't explicitly set PUBLIC_URL, as service workers will no longer be registered.
💡 Consider handling empty PUBLIC_URL
If the intent is to support empty PUBLIC_URL (defaulting to root /), consider:
-if (!(process.env.NODE_ENV === 'production' && 'serviceWorker' in navigator && process.env.PUBLIC_URL)) {
+if (process.env.NODE_ENV !== 'production' || !('serviceWorker' in navigator)) {
return;
}
+
+const publicUrlEnv = process.env.PUBLIC_URL || '';Then use publicUrlEnv instead of process.env.PUBLIC_URL in subsequent lines.
🤖 Prompt for AI Agents
In @templates/react-ts/template/src/serviceWorker.ts around lines 29 - 31, The
guard currently requires process.env.PUBLIC_URL to be truthy which skips service
worker registration when PUBLIC_URL is unset/empty; change the check to
normalize PUBLIC_URL first (e.g., compute a publicUrlEnv variable that defaults
to '/' or to new URL('.', window.location.href).toString() when
process.env.PUBLIC_URL is falsy) and then use that publicUrlEnv in the if
condition and later code paths that reference PUBLIC_URL so registrations still
occur for the root/public default while preserving explicit PUBLIC_URL behavior.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.