✨ [devext] revamp extension settings #2250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 16, 2023
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
Codecov Report
@@ Coverage Diff @@
## benoit/developer-extension--revamp-communication #2250 +/- ##
====================================================================================
- Coverage 94.09% 93.99% -0.10%
====================================================================================
Files 201 201
Lines 6083 6083
Branches 1347 1347
====================================================================================
- Hits 5724 5718 -6
- Misses 359 365 +6 see 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
BenoitZugmeyer
force-pushed
the
benoit/developer-extension--settings-scoped-by-panel
branch
from
19606a0 to
8135d2f
Compare
bcaudan
approved these changes
May 17, 2023
Comment on lines
+52
to
+64
| // https://legacy.reactjs.org/docs/hooks-faq.html#is-there-something-like-forceupdate | ||
| const [, forceUpdate] = useReducer((x: number) => x + 1, 0) | ||
|
|
||
| useEffect(() => { | ||
| const subscription = onSettingsChange.subscribe(forceUpdate) |
There was a problem hiding this comment.
💭 thought: It is not clear to me why we need to do that, maybe adding a comment could help future reviewer as well
BenoitZugmeyer
force-pushed
the
benoit/developer-extension--revamp-communication
branch
from
3ec53f6 to
fb22036
Compare
BenoitZugmeyer
force-pushed
the
benoit/developer-extension--settings-scoped-by-panel
branch
from
45fb402 to
a256c2a
Compare
Base automatically changed from
benoit/developer-extension--revamp-communication
to
main
May 17, 2023 13:04
This commit changes the way we define network rules to make sure they only apply on the tab where the devtools panel is open. When the devtools tab is closed, rules are cleared.
This commit removes the background store usage for settings.
Get rid of the store defined in the background script as it is not used anymore.
BenoitZugmeyer
force-pushed
the
benoit/developer-extension--settings-scoped-by-panel
branch
from
a256c2a to
2573add
Compare
BenoitZugmeyer
deleted the
benoit/developer-extension--settings-scoped-by-panel
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The extension settings behaved a bit unexpectedly:
Changes
This PR moves the settings logic from the background script to the devtools. No more "store", now all settings are loaded from the extension storage when the devtools is open. Every time a setting changes, it is written to the extension storage without overriding other settings. Devtools instances are not subscribing to settings updates,
so it is possible to edit the settings in a devtools instance independently from other instances, but still have persistence. This mimics how Chrome devtools settings are working.
Because the network rules are not driven by a global "store" anymore, they can be activated per tab, thanks to the devtools connection.
Testing
I have gone over the contributing documentation.