-
Notifications
You must be signed in to change notification settings - Fork 180
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
Supporting manifest v3 on webext-redux #282
Conversation
The goal is to use event to sync content-scripts and background-script store by doing the following steps: 1. Content-script load in the tab/page and request the latest state available to the background-script 2. Background script will broadcast any changes to the store 3. Background script will initialise with the latest state available in chrome.storage (Implemented outside of the library for the time being) and broadcast the state to all content-scripts running in Chrome
Following the great example on #283 (comment), I want to share we went live last week after several weeks of QA. Our extension is https://chrome.google.com/webstore/detail/skimlinks-editor-tool/gmfcchdbgideoiehadmlmpnbhdcjoaih?hl=en We release the PR using the following package https://www.npmjs.com/package/@eduardoac-skimlinks/webext-redux |
Hi, @eduardoacskimlinks thanks for building it to support MV3! I used this PR and found that I get store from Popup but the state of Popup didn't sync with the state of the background script. Let's say, I made a React component and dispatch a thunk action to the background script to fetch API, but after the background script received the API response and updated the redux state, the state of Popup was not updated. I am sure the previous |
@eduardoacskimlinks Thanks for building this! Where should the initializeExtension() code go? |
Hi @nasonlee28, thanks for the support. If you moved to Manifest V3 in your extension then Regarding your issue, it seems similar to the one we faced in the content-script. However, I need more details on the implementation to guide you through any potential solutions. For example, is your extension open source? @z0ccc The setup is the same as before regarding |
@eduardoacskimlinks Hi, thanks for the reply. I'm confused as to where clearState, saveState, etc come from. Are these functions I need to write myself? |
Hi @eduardoacskimlinks, thanks for replying to me. I have a reducer and action file: import merge from 'lodash/merge';
export const openPopup = () => {
return (dispatch) => {
setTimeout(() => {
dispatch({ type: 'OPEN_POPUP' });
}, 5000);
};
};
export const uiOpenPopup = () => {
return {
type: 'UI_OPEN_POPUP',
};
};
const initialState = {
isPopupOpened: false,
};
export default (state = initialState, action) => {
const { type } = action;
switch (type) {
case 'OPEN_POPUP':
return merge({}, state, { isPopupOpened: true });
default:
return state;
}
}; And I applied the applyMiddleware(alias({ UI_OPEN_POPUP: openPopup })) Here is how my popup dispatch action: // In my react component
const Popup = () => {
useEffect(() => {
store.dispatch(uiOpenPopup());
}, []);
const isPopupOpened = useSelector((state) => state.test.isPopupOpened);
return <>{`isPopupOpened: ${isPopupOpened}`}</>;
};
const store = await getProxyStore();
ReactDOM.render(
<Provider store={store}>
<Popup />
</Provider>,
document.getElementById('app')
); How I get store in Popup script: import { Store } from '@eduardoac-skimlinks/webext-redux';
let store = null;
export const getProxyStore = async () => {
if (!store) {
store = new Store();
}
await store.ready();
return store;
}; The first time I opened the popup, I saw |
Not sure if it is relative to MV3 or redux, I found that my console panel of popup shows the redux logs by |
@nasonlee28 Sorry for the late reply, we were closing the quarter, and I didn't have much time available. Regarding the issue, it seems precisely the one we faced on the content scripts on our side that drove us to create this PR. Have you used the persistence storage that I describe in the PR description? It's likely that the service worker goes to sleep and sends a new message with the original state causing the pop-up to reset to the initial store state otherwise. I recommend searching on your bundle disabling minifying for the function Finally, I don't discard that it may be an error on the PR, so we need to do some debugging to understand where the problem comes from Setup or the code itself. |
Hi, guys! First of all, @eduardoacskimlinks thank you for your excellent work.
Nonetheless, I’ve prepared a fix for it. We could notify Popup together with content-scripts inside To test it:
Screen.Recording.2022-07-13.at.19.54.39.mov |
@RomanSavarin Thanks for contributing to the PR and providing great context. I will review your proposed changes and try to come back to you in a couple of days. |
Broadcast store changes to popup
@nasonlee28 I have released the changes from @RomanSavarin into the following version Kudos to @RomanSavarin, for the great explanation and example. There were helpful to try the changes |
@eduardoacskimlinks, @RomanSavarin, @nasonlee28 , @z0ccc & everyone else who helped to make this happen: THANK YOU! I just replaced the dependency with your package and everything works as expected so far. I will do some more testing, but that's saving me and others so much time. |
@svrnm Glad to hear. Hopefully, we can keep this project alive even if it's through a fork |
After extensive experimentation, I found Redux isn't the most effective solution for scaling our Chrome extension. Check out my article for insights. Stay tuned for the follow-up on building a robust React UI extension without Redux. (@eduardoacskimlinks is company account 😄) |
following up on this as well: things looked like they worked for a while but things are still not stable/breaking from time to time (even when using |
@svrnm That also happened to us; what I found, in general, is the broadcast effectively works, but we found two significant situations when it becomes cranky:
I would like to hear about your challenges and learn more about other developers experiences |
@eduardoacskimlinks we never really could isolate a root cause of the issue, all we get is complaints by users of the extension (https://github.com/svrnm/demomonkey) that storing configurations is broken randomly (which let us to a similar assumption that there is somehow something not OK with the service workers). For some time we assumed that bugs in chrome are responsible (see https://bugs.chromium.org/p/chromium/issues/detail?id=1316588 and https://bugs.chromium.org/p/chromium/issues/detail?id=1426461), but after we thought it was fixed, it now returned once again. Final Note: I have to admit that when I created that extension years ago I had some fundamental misunderstandings of React&Redux and a lot of stuff that shouldn't go through that mechanism, is flowing through it. It worked for multiple years just fine, so I never touched it 🤷♂️ cc @clintonfray |
@eduardoacskimlinks, The issues we experiencing has the following symptoms:
|
Thanks for sharing for additional coverage. This is helpful because it validates that we are experiencing the same issues. |
Hey @eduardoacskimlinks, thanks for working on this! I've cherry-picked these changes into #297 to make some additional fixes. Closing this PR in favor of #297. |
That's fantastic news, @SidneyNemzer! I appreciate you taking the time to incorporate my work and make the additional fixes. I'll keep an eye on #297. My personal GitHub handler moving forward is @EduardoAC, so feel free to tag me if there's anything else I can do to help. |
Supporting manifest v3 within webext-redux
The goal is to move away from connecting architecture into an event-driven model where the state changes will be broadcast across the tabs, allowing the backend-script store to sync within the different content-script.
How has the architecture changed?
We have adjusted the architecture by introducing an initialisation and broadcast mechanism which will follow three parts:
Resources
Fix #244