-
-
Notifications
You must be signed in to change notification settings - Fork 254
Use Bswup.skipWaiting in force-update process of Boilerplate (#10922) #10923
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
Use Bswup.skipWaiting in force-update process of Boilerplate (#10922) #10923
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant window.BitBswup
participant window.BitBswupProgress
User->>App: Call forceUpdate()
App->>window.BitBswup: skipWaiting()
alt skipWaiting returns truthy
App-->>User: Return (early exit)
else skipWaiting returns falsy
App->>window.BitBswupProgress: configure({ autoReload: true })
App->>window.BitBswup: checkForUpdate()
App-->>User: Return
end
Assessment against linked issues
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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: 2
🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/app.ts (1)
84-85: The update logic is correct, but consider adding error handling.The sequence of configuring the progress handler before checking for updates is logical and correct.
Consider wrapping these operations in error handling for robustness:
- bswupProgress.config({ autoReload: true }); - bswup.checkForUpdate(); + try { + bswupProgress.config({ autoReload: true }); + bswup.checkForUpdate(); + } catch (error) { + console.error('Failed to configure or check for updates:', error); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/app.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/app.ts (1)
78-78:❓ Verification inconclusive
Verify that all callers handle the async method signature change.
The method signature change from synchronous to asynchronous is a breaking change. Ensure that all existing callers of
forceUpdate()have been updated to properly await the call or handle the returned Promise.
🏁 Script executed:
#!/bin/bash # Description: Find all callers of forceUpdate method to verify they handle the async nature # Expected: All callers should either await the call or handle the Promise echo "Searching for calls to App.forceUpdate()..." rg -A 3 -B 1 "\.forceUpdate\(\)" --type ts --type js --type tsx --type jsx echo "Searching for calls to forceUpdate()..." rg -A 3 -B 1 "forceUpdate\(\)" --type ts --type js --type tsx --type jsxLength of output: 393
Verify Async Breaking Change for App.forceUpdate
The
forceUpdatemethod was changed from synchronous:public static forceUpdate() { … }to asynchronous:
public static async forceUpdate() { … }This is a breaking change to the public API. Ensure that every invocation has been updated to handle the returned
Promise(e.g. usingawaitor.then).• Run this search to locate all call sites:
grep -RIn --include=\*.{ts,tsx,js,jsx} -C2 "forceUpdate(" .• Verify in each file that calls to
App.forceUpdate()orforceUpdate()are properlyawaited or chained.
• Update any missingawait/.thento avoid unhandled Promise issues.
closes #10922
Summary by CodeRabbit