8000 fix(graphql): pin the version of assets in the graphql playground by rymnc · Pull Request #2993 · FuelLabs/fuel-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(graphql): pin the version of assets in the graphql playground #2993

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

Merged
merged 5 commits into from
May 20, 2025

Conversation

rymnc
Copy link
Member
@rymnc rymnc commented May 6, 2025

Linked Issues/PRs

playground broken across various networks due to async-graphql/async-graphql#1703

Description

2 main things -

  • we generate the html body once, and reuse it across calls to /v1/playground. previously we were re-rendering the template every time, which might not be a good solution
  • in the raw html generated, we pin the versions of the js and css assets distributed by unpkg

Checklist

  • Breaking changes are clearly marked as such in the PR description and changelog
  • New behavior is reflected in tests
  • The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

@rymnc rymnc requested review from a team and xgreenx as code owners May 6, 2025 03:41
@rymnc rymnc added the bug Something isn't working label May 6, 2025
@rymnc rymnc requested review from Dentosal and MitchTurner as code owners May 6, 2025 03:41
@rymnc rymnc self-assigned this May 6, 2025
@rymnc rymnc requested a review from netrome as a code owner May 6, 2025 03:41
Comment on lines +396 to +403
let raw_html = raw_html.replace(
"https://unpkg.com/graphiql/graphiql.min.js",
"https://unpkg.com/graphiql@3/graphiql.min.js",
);
let raw_html = raw_html.replace(
"https://unpkg.com/graphiql/graphiql.min.css",
"https://unpkg.com/graphiql@3/graphiql.min.css",
);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we could get away with a singular replace, i.e

Suggested change
let raw_html = raw_html.replace(
"https://unpkg.com/graphiql/graphiql.min.js",
"https://unpkg.com/graphiql@3/graphiql.min.js",
);
let raw_html = raw_html.replace(
"https://unpkg.com/graphiql/graphiql.min.css",
"https://unpkg.com/graphiql@3/graphiql.min.css",
);
let raw_html = raw_html.replace(
"https://unpkg.com/graphiql/graphiql.min.",
"https://unpkg.com/graphiql@3/graphiql.min.",
);

i would rather have it be explicit about which assets we are pinning, and since this part of the code only runs once, it seems ok imo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@rymnc rymnc added the graphql-api Affects API of the GraphQL label May 6, 2025
Comment on lines +396 to +403
let raw_html = raw_html.replace(
"https://unpkg.com/graphiql/graphiql.min.js",
"https://unpkg.com/graphiql@3/graphiql.min.js",
);
let raw_html = raw_html.replace(
"https://unpkg.com/graphiql/graphiql.min.css",
"https://unpkg.com/graphiql@3/graphiql.min.css",
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@xgreenx xgreenx merged commit e7ca966 into master May 20, 2025
33 of 34 checks passed
@xgreenx xgreenx deleted the fix/graphql-playground branch May 20, 2025 15:40
Dentosal added a commit that referenced this pull request May 26, 2025
## Version 0.44.0

### Breaking
- [2887](#2887): Bump Rust
version to `1.85.0`.
Starting with this release, newly generated WASM state transition
functions are no longer compatible with old versions of the `fuel-core`.
So, the change breaks forward compatibility for the network and each
node should start to use a new `fuel-core` release.
- [2943](#2943): Registry root
calculation for compression tables no longer accounts for the evictor
cache.
- [2947](#2947): Upgrade to
2024 Rust edition.
- [2958](#2958): Changed
return type of `balance` endpoint from `u64` to `u128`
- [3002](#3002): Update
`fuel-vm` to `0.61.1`. In doing this, we've changed Receipts to use the
`SubId` scalar type for sub asset IDs.

### Added
- [2954](#2954): Made
`registry` mod public in `fuel-core-compression`
- [2956](#2956): Add
excluded_contract filter to extract of transaction from TxPool.
- [2994](#2994): Simple
makefile with basic commands.
- [3004](#3004): Additional
error logs for Ethereum provider failures.

### Changed
- [3021](#3021): Updated
fuel-vm to 0.62.0, see
https://github.com/fuelLabs/fuel-vm/releases/v0.62.0

### Fixed
- [2969](#2969): Ensure that
vm heap memory is zeroed out on rellocation after `reset`. Adds support
for `GM::GetGasPrice` Bumps `fuel-vm` to `0.60.2`.
- [2984](#2984): Fix client
coins endpoint so that passing `None` for `asset_id` no longer defaults
to `AssetId::default()` but correctly returns all asset types.
- [2987](#2987): Make txpool
pre-conf broadcast conditional on there being some txs in the list
- [2989](#2989): Prevent
syncing compression database from genesis if override cli arg
`--da-compression-starting-height` is provided.
- [2992](#2992): Make sure
assemble tx doesn't count message data inputs as spendable inputs for
covering fee
- [2993](#2993): Pin the
graphiql playground to v3, and cache the result to be reused across
multiple calls to render the playground.

### Removed
- [2955](#2955): Remove
unnecessary lifetime constraints on fuel-core-client.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working graphql-api Affects API of the GraphQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0