[graphiql] Refreshing page can erroneously create new tabs

[simhnna]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

If you set initial headers and disable shouldPersistHeaders graphiql creates a new tab although it shouldn't

Tabs aren't stored with headers, but the getDefaultTabState function doesn't know about the shouldPersistHeaders flag. So it compares a stored tab without headers with the headers that are passed to graphiql and decides to create a new tab

Expected Behavior

Refreshing the page shouldn't create a new tab

Steps To Reproduce

In this repo edit the renderExample.js setting the

var parameters = {
  headers: '{"foo": "bar"}' 
};

and shouldPersistHeaders: false,

### Module pattern

- [ ] graphiql-umd
- [ ] graphiql-esm
- [ ] graphiql-commonjs

### Environment

_No response_

### Anything else?

_No response_

[simhnna]

I would propose passing shouldPersistHeaders to getDefaultTabState and using that to decide if the headers should be considered while computing the hash.

If that's accepted I'll prepare a merge request

[simhnna]

Oh I just found #2145 adding that would probably also fix my issue

[thomasheyenbrock]

I would propose passing shouldPersistHeaders to getDefaultTabState and using that to decide if the headers should be considered while computing the hash.

Fixing the behavior of the headers props like you described here would still be much appreciated! Do you want to open a PR for that?

[acao]

Here is a simple reproduction of this bug:
https://codesandbox.io/s/graphiql-bug-2825-kxx2j7?file=/index.html

it only requires setting a headers value, and refreshing the page

[acao]

a clue - this bug goes away when shouldPersistHeaders is enabled.

it seems that the tab state context on init mismatches the previous state, and assumes the query parameters or user input of headers are indicating a new tab state

update: i have a "fix", however I can't get the e2e suite to re-create this bug!

[acao]

if any users want to confirm the fix, try this canary release:

• npm: graphiql@3.0.5-canary-f6e35dd1.0
• unpkg: https://unpkg.com/graphiql@3.0.5-canary-f6e35dd1.0/graphiql.js

[acao]

@markedwards can you try these patch versions and confirm the fix works for you, at least for headers creating new tabs (I will address persisting user defined settings next)? I have no one to review my PRs right now haha

[markedwards]

Sorry, haven't got to it yet. I'll try to test today or tomorrow.

It seems to be fixed on https://deploy-preview-3377--graphiql-test.netlify.app

[markedwards]

@acao Does not seem to be fixed when using https://unpkg.com/graphiql@3.0.5-canary-f6e35dd1.0/graphiql.js.

[acao]

@markedwards the fix has already been released, can you try it with the latest? I was able to reproduce the bug and eliminate the bug with this release

[markedwards]

Okay, well I can no longer reproduce multiple <untitled> tabs spawning (one per refresh). However, I still get one <untitled> tab on refresh. My expectation is, if I already have tabs, the most recent one should be shown on refresh instead of spawning an empty one.

Oddly, I no longer reproduce the multiple tab issue, even on https://unpkg.com/graphiql@3.0.0/graphiql.min.js. Did the behavior of 3.0.0 change?

[acao]

hmm, it seems the issue wasn't completely resolved then. 3.0.0 was released a while ago so the behavior shouldn't have changed. it would've been the last graphiql patch version that resolves the issue, 3.0.5

[acao]

@markedwards what happens when you try deleting all the tabs and history, and then try to reproduce the bug? or better yet, if you clear local storage state and try reproducing it? lingering local storage states may cause this issue to seem like it's still an issue. I cannot seem to get a tab to appear on refresh anymore

[markedwards]

Wiping local storage doesn't seem to have any effect, the behavior is exactly the same. I see no difference between 3.0.0 and 3.0.5, they both spawn empty tabs on refresh. I can sometimes get more than one to spawn on both versions actually, but I'm not exactly sure when this happens. I think it has something to do with persistent headers.

[acao]

@markedwards this example clears it up:

No new tabs are created when shouldPersistHeaders is set to true:

https://codesandbox.io/s/graphiql-bug-2825-kxx2j7?file=/index.html

But when you disable this value, and refresh, you get new headers. So the issue is not fixed.

My bugfix works for the first case, but for the second case, causes a mismatch, because headers are set statically, but not persisted.

[acao]

I think the best option is to remove shouldPersistHeaders as an option and always make it true, and remove the user setting.

also this issue is not apparent when you use defaultHeaders instead of headers. headers is meant for programatically setting the current headers state, whereas defaultHeaders works like defaultQuery, and is ignored when you already have values set.

if you want to set default headers for every query, and make them a transparent choice, set the headers in your fetcher statically

[markedwards]

I’m reproducing this without any settings being overridden at all. I’m not setting headers, or changing any default setting.

[fragm3]

Even I am running into the same issue without any settings being changed.