smapiot / piral

🚀 Framework for next generation web apps using micro frontends. ⭐️ Star to support our work!
https://piral.io
MIT License
1.71k stars 127 forks source link

Changes to browser's session store for Piral debug-utils are ignored #590

Closed Robbson closed 1 year ago

Robbson commented 1 year ago

Bug Report

Changes to browser's session store for Piral debug-utils are ignored when running the Piral instance (using piral debug) the first time or when opening it in a new browser tab.

Prerequisites

Steps to Reproduce

  1. Create a new Piral instance app using piral new --target my-app
  2. Start the app using npx piral debug and choose webpack5 as bundler
  3. Add sessionStorage.setItem('dbg:view-state', 'off'); anywhere to the index.tsx
  4. Open http://localhost:1234 in a new browser tab
  5. Open the browser's console and you will see the state debug output even though you have disabled it. This happens every time you start a new session (not using page refresh but opening a new browser tab). Looking in the browser's application state, you can see, the value is correctly set but Piral ignores it.

Expected behavior

My changes to the debug settings should always work, so there should be no state debug output in the console.

Actual behavior

See step 5.

Possible Origin/Solution

This behaviour has probably been introduced in Piral 0.14.30 when the Piral debug-utils got a new persistence feature (it worked just fine in 0.14.28).

FlorianRappl commented 1 year ago

I just tried this. It works. Sure you are doing the correct thing? All I did was paste

sessionStorage.setItem('dbg:view-state', 'off')

and hit F5. No more view state output.

Please provide a MWE - otherwise I don't think this is a Piral issue, but rather something on your particular application or instance (maybe you override or change somehow sessionStorage?!).

Reload

Robbson commented 1 year ago

Hitting F5 doesn't clear the session store. You have to open it in a new tab.

FlorianRappl commented 1 year ago

Hitting F5 doesn't clear the session store. You have to open it in a new tab.

? I am not sure how this is related. The issue is about that the session store value is not picked up. I just tried it and it is picked up.

Robbson commented 1 year ago

No, it's not, when you open it in a new tab (so using a new session store). I used the same steps from above using the latest Piral 0.15.8. An I use Chrome.

FlorianRappl commented 1 year ago

No, it's not, when you open it in a new tab (so using a new session store).

I think we have the problem here; you think the sessionStorage works across tabs. It does not. Each tab has a new session. This has nothing to do with Piral - that's just how browsers work.

Robbson commented 1 year ago

Of course it is related. When you press F5, the session store (with the right value) still exist when the application gets reloaded. So Piral can pick it up in that case.

Robbson commented 1 year ago

No, it's not, when you open it in a new tab (so using a new session store).

I think we have the problem here; you think the sessionStorage works across tabs. It does not. Each tab has a new session. This has nothing to do with Piral - that's just how browsers work.

Where I did say that the sessionStorage works across tabs? That's confusing.

FlorianRappl commented 1 year ago

Where I did say that the sessionStorage works across tabs? That's confusing.

You did above when you suggested that opening in a new tab should still take the value set in the sessionStorage closed tab.

Again, I tested it - it is picked up. There is no need to close / open a new tab. As mentioned, the browser opens a new sessionStorage per tab, so the previous value is of course not preserved. My test has shown that the value is picked up, so the reported issue is not an issue.

See also the video above.

Coming back to the OP: If your issue is that you want to prevent that output for everyone then just apply that value before importing piral or piral-core. Assuming it's in index.tsx you can just import './globals' on the first line, where global.ts could have only this one line containing sessionStorage.setItem. It's not recommended to do that, but surely you can do it like that. That the storage is read out on import is the intended way.

Robbson commented 1 year ago

You did above when you suggested that opening in a new tab should still take the value set in the sessionStorage closed tab.

No, I haven't at all. Please quote it!

And I have no idea why this should be the intended way as you said in your last sentence. Why should anywhone be bothered with state change debugging output when the state changes just work fine?

The Piral-Debug-Utils seems to always get imported first, that's why sessionStorage.setItem('dbg:view-state', 'off') is just too late. And furthermore, it is still ignored from Piral because for some reason the storage change listener doesn't trigger. It only triggers when you make changes to the storage from the browser.

FlorianRappl commented 1 year ago

The Piral-Debug-Utils seems to always get imported first, that's why sessionStorage.setItem('dbg:view-state', 'off') is just too late. And furthermore, it is still ignored from Piral because for some reason the storage change listener doesn't trigger. It only triggers when you make changes to the storage from the browser.

I mentioned what you need to do. You define the import order - make use of it.

Regarding the listener; that fires (as you noted), but of course - only after it was installed. So what's happening is that you

  1. import piral (so storage is read out)
  2. change the value of sessionStorage (but since storage is read out that does not matter)
  3. set up your Piral instance (which will eventually install the debug utils and thus connect the handler)

So you enter the unreported void (2). Again, either place your code above (1) or get below (3). The for the latter you could use a mounted hook in your instance.

And I have no idea why this should be the intended way as you said in your last sentence. Why should anywhone be bothered with state change debugging output when the state changes just work fine?

I am not sure I get that sentence. The setting allows anyone to define (for the current session) to enable or disable state change output. If you don't want to see this then you can use the Piral Inspector and globally disable state change output. But still, this is (and should be) a personal preference. But then again, note that this was not the issue you raised, which is a bug - but instead, it would be a feature request to, e.g., change that default.

Robbson commented 1 year ago

Yes, a feature request sounds good to me. The defaults could be set using the CLI or the pilet.json or something like that?

Indeed, you could set the defaults in the Piral Inspector. But our Pilet developers usually don't use this browser extension because they spend most (about 95%) of their time on components which are not related to Piral. When they start the Piral Emulator, the first thing they see is a pretty crowded console with hundreds of internal state changes. And I would like to avoid that.

Robbson commented 1 year ago

Regarding the listener; that fires (as you noted), but of course - only after it was installed. So what's happening is that you

  1. import piral (so storage is read out)
  2. change the value of sessionStorage (but since storage is read out that does not matter)
  3. set up your Piral instance (which will eventually install the debug utils and thus connect the handler)

So you enter the unreported void (2). Again, either place your code above (1) or get below (3). The for the latter you could use a mounted hook in your instance.

I just used a naive timer after the Piral Instance setup (of course only for testing puposes in this MWE) like this:

setTimeout(() => {
    console.log('Change Session Storage')
    sessionStorage.setItem('dbg:view-state', 'off')}
, 4000)

It still doesn't trigger the listener so futher changes to the app state would still be shown in the console.