Closed Uninen closed 3 years ago
Hi @Uninen, first of all thank you, I'm glad you like the plugin 😃.
I think it is a great idea to offer that feature, but I have doubts about the possible implementation, maybe you can help me:
I have thought two possible approaches:
localStorage
using browser settings, which would be my preferred one from the point of view that it would be the more realistic one, but it has some cons:
cy.disableLocalStorage
or cy.enableLocalStorage
once the browser has been launched. (Further info here: https://docs.cypress.io/api/plugins/browser-launch-api.html#Changing-browser-preferences)localStorage
, and some of them only allow to disable these type of features as a bundle, so, for disabling localStorage
it would be necessary to also disable cookies, etc. which maybe could be confusing for a method named "disableLocalStorage".localStorage
has been disabled replacing it by a fake object when using cy.disableLocalStorage
, allowing to restore it in any moment using cy.enableLocalStorage
. This method would have the advantage that it could be used on the fly, but it has also some cons:
localStorage
, so every browser should be investigated to found the better way to simulate that behaviour on it, and then the plugin should use one method or another depending on the current browser.localStorage
, it couldn't be even possible to simulate that behaviour. I mean, maybe some browsers remove the full localStorage
property when disabled, and set it as a read-only
object when enabled, etc.I think I will investigate the first one approach deeper, but let me know your opinion please.
While using the browser settings to disable localStorage
sounds optimal, I'm pretty sure that path leads to more headaches in the long term, at least until Cypress has a better API for doing that with all supported browsers.
The second option sounds better, because it's more practical, and allows for more freedom when writing tests. We could for example always throw by default from setItem()
but allow easy override if you want to return something else instead. For most of the use cases I can think of where you test the availability of localStorage
this should be enough.
My current tests work like this, which was easy and clean to implement:
cy.on('window:before:load', win => {
cy.stub(win.localStorage, 'setItem').throws()
})
(This should also cover almost all real code as at for example MDN docs explicitly state that "developers should make sure to always catch possible exceptions from setItem()" and based on my experience that's how the availability usually gets tested as well.)
Hi again @Uninen. Finally I have used your recommended approach, but I have also stubbed the getItem
, removeItem
and clear
methods after checking that Chrome, Safari and Firefox are throwing errors also in those cases.
I have no implemented the enableLocalStorage
command because the disableLocalStorage
one has only effect for the current test, so it is enabled automatically again in the next one. This detail, added to the fact that the name enableLocalStorage
could be a little bit confusing, because it would only enable localStorage
if it had been disabled using the disableLocalStorage
command made me to not implement it for the moment.
I have added also the possibility to use a custom error through the withError
option in the disableLocalStorage
command, as in:
cy.disableLocalStorage({
withError: new Error("Disabled by cypress-local-storage")
});
In hope it will be useful 🙂
Thank you for your contribution!, I will mention it in the release notes and changelog.
This is superb - absolutely spot on.
Thank You for your work @javierbrea 🙏
Is your feature request related to a problem? Please describe.
When working with localStorage it's almost always necessary to handle exceptions as well. IT would be useful if this plugin would offer a way to easily disable the local storage.
Describe the solution you'd like
Maybe
cy.disableLocalStorage()
andcy.enableLocalStorage()
commands, for example.Describe alternatives you've considered
Don't actually know any easy alternative at the moment (hence the ticket).
Additional context
I really like this plugin, Thank You for your work 👍