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

Pin graphql-voyager to 1.3.0, fix #1152 #1153

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

nightroman
Copy link
Contributor

No description provided.

@Shane32
Copy link
Member

Shane32 commented Oct 1, 2024

Let's pin 2.0.0; it has been in use for a year now, right?

@nightroman
Copy link
Contributor Author

Let's pin 2.0.0; it has been in use for a year now, right?

But it does not work for me with 2.0.0.
1.3.0 is the maximum that works.

@nightroman
Copy link
Contributor Author

nightroman commented Oct 1, 2024

Take a looks at 1.3.0 (works fine) and 2.0.0 (Loading... infinitely)
2-cases.zip

@Inok
Copy link

Inok commented Oct 1, 2024

Let's pin 2.0.0; it has been in use for a year now, right?

No, it hasn't been: in 2.0, voyager.min.js was removed, so https://cdn.jsdelivr.net/npm/graphql-voyager/dist/voyager.min.js (without version) still points to the 1.3 version.

However, the corresponding CSS file is still present in 2.0, so there was CSS from 2.0 and JS from 1.3 for a year.
And in 2.1, it seems, the CSS finally became incompatible with the JS v1.3.

Switching to the new voyager.standalone.js from 2.0 (supersedes voyager.min.js) doesn't help because of some configuration changes in 2.0.

@Shane32
Copy link
Member

Shane32 commented Oct 1, 2024

Ok sounds good. Let's add the sha hash to the tag to ensure it does not change in the future, and I'll merge it.

@nightroman
Copy link
Contributor Author

Ok sounds good. Let's add the sha hash to the tag to ensure it does not change in the future, and I'll merge it.

Sorry, I am not sure what this is. Is it something I should do?

@Shane32
Copy link
Member

Shane32 commented Oct 1, 2024

@Shane32
Copy link
Member

Shane32 commented Oct 1, 2024

Probably some website like this can calculate it.

https://www.srihash.org/

@alexander-jesner-AP
Copy link

I assume the hosting code needs to be updated for the 2.0 version to work?
Like this: https://github.com/graphql-kit/graphql-voyager/blob/c644fb529f8aa1f987c505aef4527207a0c752fd/example/cdn/index.html#L31-L51

@nightroman
Copy link
Contributor Author

nightroman commented Oct 2, 2024

@alexander-jesner-AP Yes, I think so and hope this will be eventually the case, the proper fix. That's why I mentioned the same example in #1152

@Shane32 I have engaged for the quick fix of the regression, to make things working again. I now see it's getting more involved and it's not really my area of expertise. At the moment I am not able to implement what you are requesting. Thank you for your help.

@Shane32
Copy link
Member

Shane32 commented Oct 2, 2024

I'll issue a release with it pinned to 1.3.0

If someone wants to post a PR with it properly upgraded to 2.x, we can merge that in subsequently.

@Shane32 Shane32 merged commit a84c553 into graphql-dotnet:master Oct 2, 2024
7 checks passed
@Shane32 Shane32 added this to the 8.0.3 milestone Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants