Skip to content
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

Only store persistable state on unmount #105

Merged
merged 1 commit into from
Mar 9, 2016
Merged

Conversation

leebyron
Copy link
Contributor

@leebyron leebyron commented Mar 9, 2016

Currently we write to persistable state on each change to any persistable state including the current query and variables as well as some the resizable elements of the UI. This suggests replacing that with only saving persistable state when the component is about to unmount.

This fixes a problem where opening multiple instances of GraphiQL at the same time can result in one query appearing across all views. Now, only the last window closed will save the query. This also fixes a frustration where accidentally closing a tab and then editing another instance of GraphiQL renders the accidentally closed tab harder to recover.

This also fixes a performance issue where setting into storage can be slightly expensive and we were performing this on every keystroke.

The primary drawback is the potential for lost state in the case of an unexpected browser cache, since this no longer saves to persistable state optimisitically. I believe that's worth the gains and a minor issue relative to the accidental tab-closing issue.

Currently we write to persistable state on each change to any persistable state including the current query and variables as well as some the resizable elements of the UI. This suggests replacing that with only saving persistable state when the component is about to unmount.

This fixes a problem where opening multiple instances of GraphiQL at the same time can result in one query appearing across all views. Now, only the last window closed will save the query. This also fixes a frustration where accidentally closing a tab and then editing another instance of GraphiQL renders the accidentally closed tab harder to recover.

This also fixes a performance issue where setting into storage can be slightly expensive and we were performing this on every keystroke.

The primary drawback is the potential for lost state in the case of an unexpected browser cache, since this no longer saves to persistable state optimisitically. I believe that's worth the gains and a minor issue relative to the accidental tab-closing issue.
leebyron added a commit that referenced this pull request Mar 9, 2016
[RFC] Only store persistable state on unmount
@leebyron leebyron merged commit 7537a69 into master Mar 9, 2016
@leebyron leebyron deleted the store-on-unmount branch March 9, 2016 02:09
@leebyron leebyron changed the title [RFC] Only store persistable state on unmount Only store persistable state on unmount Mar 9, 2016
acao pushed a commit to acao/graphiql that referenced this pull request Jun 1, 2019
Teach Travis to cache yarn-installed modules
acao pushed a commit to acao/graphiql that referenced this pull request Jun 5, 2019
Make foot-gun-shield work from subdirectories too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants