graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.12k stars 1.73k forks source link

fix: remove deprecated Function.prototype.caller reference #3806

Closed simmerer closed 2 weeks ago

simmerer commented 2 weeks ago

See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Deprecated_caller_or_arguments_usage

Screenshot 2024-11-05 at 3 08 01 PM

While it's helpful during development to have as much information as possible about which component doesn't have the necessary context, it's very not helpful to have the error message itself throw a blocking error due to this deprecated usage 😅

In a future PR, it might be nice to rename the caller argument found throughout GraphiQL packages to something like callerFn for clarity, making it clear at a glance that it's a custom arg and not an attempt to access Function.prototype.caller.

changeset-bot[bot] commented 2 weeks ago

🦋 Changeset detected

Latest commit: 91fde2bf7bb3eac9788e568b536d931638a9da2e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | ------------------------------ | ----- | | @graphiql/react | Minor | | @graphiql/plugin-code-exporter | Major | | @graphiql/plugin-explorer | Major | | graphiql | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

linux-foundation-easycla[bot] commented 2 weeks ago

CLA Signed

The committers listed above are authorized under a signed CLA.

simmerer commented 2 weeks ago

Related issues:

acao commented 2 weeks ago

thanks @simmerer ! I am currently in the process of migrating most of this logic to a lower service level, and will be removing this usage of caller. this will be very helpful for the time being, thank you!

noting that this will cause bugs when users accidentally double tap the execute button, or execute multiple queries quickly in serial, as the usage of caller is meant to also uniquely identify the request being executed, so that previous in-flight requests can be cancelled iirc. but this is something I am already accommodating for in the rewrite using unique IDs, and for now I think we can ensure the button is fully disabled

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 65.32%. Comparing base (7a59187) to head (91fde2b). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/graphiql-react/src/utility/context.ts 0.00% 1 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/graphql/graphiql/pull/3806/graphs/tree.svg?width=650&height=150&src=pr&token=XdBgmoWsyv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql)](https://app.codecov.io/gh/graphql/graphiql/pull/3806?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql) ```diff @@ Coverage Diff @@ ## main #3806 +/- ## ======================================= Coverage 65.32% 65.32% ======================================= Files 122 122 Lines 7003 7003 Branches 2260 2260 ======================================= Hits 4575 4575 Misses 2411 2411 Partials 17 17 ``` | [Files with missing lines](https://app.codecov.io/gh/graphql/graphiql/pull/3806?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql) | Coverage Δ | | |---|---|---| | [packages/graphiql-react/src/utility/context.ts](https://app.codecov.io/gh/graphql/graphiql/pull/3806?src=pr&el=tree&filepath=packages%2Fgraphiql-react%2Fsrc%2Futility%2Fcontext.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql#diff-cGFja2FnZXMvZ3JhcGhpcWwtcmVhY3Qvc3JjL3V0aWxpdHkvY29udGV4dC50cw==) | `84.61% <0.00%> (ø)` | |
acao commented 2 weeks ago

I'm going to call this good to go for now, as the full resolution will be introduced in 4.x soon! thank you!