-
Notifications
You must be signed in to change notification settings - Fork 2k
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 trigger "one instance of graphql" error in DEV environments. #1174
Conversation
This ensures the additional checking for multiple graphql instances only occurs in non-production builds. Not only does this improve the performance for production builds, it also solves a spurious error case for minified builds: Since this additional check compares the `.name` of the constructors as a proxy for determining if two classes are representationally equal, minified builds may result in spurious false-positives if class names are minified to single-character names and those characters are likely to conflict. This doesn't require `process` to exist (defaulting to the non-production mode), but bundlers like Webpack should consider using a stripping transform (see: https://webpack.js.org/guides/production/#specify-the-environment) in production builds if they also use a minifier.
496c1a0
to
9b7a292
Compare
That's a big blocker for using new features of graphql-js, any idea when the next release will be? Thank you! 👍 |
Would love to see this released too as this causes problems that only appear in prod builds. |
Also waiting for this, graphiql stopped working for us completely in production. |
Struggling with this issue too. |
When can we expect this to be released? |
Same issue, please release :) |
Same issue |
FYI for those that desperately need this, we've had success rolling back to |
Really just need the updated GraphQLErrors object with extensions |
+1 need GraphQLErrors as well! Thanks!! |
If you guys are having issues with GraphiQL, you can override qraphql dependency in package.json using the most recent version, like so
It solved the problem for me. |
@nparse is this making it into the next release? Another but maybe related problem is that
|
Same problem that @BerndWessels here... Using MeteorJS (using I try with graphql 0.12.3 and 0.12.0 |
This is fixed in 0.13/0.13.1
…On Thu, Feb 22, 2018 at 4:36 PM, Esteban Ruiz de Galarreta < ***@***.***> wrote:
Same problem that @BerndWessels <https://github.com/berndwessels> here...
Using MeteorJS (using uglify )
I try with graphql 0.12.3 and 0.12.0
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1174 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AALHBWQhvnIVgUuRdE4L-O-MdkWuLlirks5tXYmOgaJpZM4RJAhL>
.
|
An improvement beyond graphql#1174, for non-production environments that enable minification, such as NODE_ENV='staging'. Since graphql-js goes to the trouble of providing a Symbol.toStringTag property for most of the classes it exports, and that string is immune to minification (unlike constructor.name), we should use it for error checking in instanceOf(value, constructor) whenever the constructor provides a tag, falling back to constructor.name and value.constructor.name for constructors that do not define Symbol.toStringTag (as before). Motivating issue/investigation: apollographql/apollo-client#7446 (comment)
I'm using 15.5.0 in a React Native project (Expo) and this is causing the built binaries to crash with the |
An improvement beyond graphql#1174, for non-production environments that enable minification, such as NODE_ENV='staging'. Since graphql-js goes to the trouble of providing a Symbol.toStringTag property for most of the classes it exports, and that string is immune to minification (unlike constructor.name), we should use it for error checking in instanceOf(value, constructor) whenever the constructor provides a tag, falling back to constructor.name and value.constructor.name for constructors that do not define Symbol.toStringTag (as before). Motivating issue/investigation: apollographql/apollo-client#7446 (comment)
This ensures the additional checking for multiple graphql instances only occurs in non-production builds. Not only does this improve the performance for production builds, it also solves a spurious error case for minified builds:
Since this additional check compares the
.name
of the constructors as a proxy for determining if two classes are representationally equal, minified builds may result in spurious false-positives if class names are minified to single-character names and those characters are likely to conflict.This doesn't require
process
to exist (defaulting to the non-production mode), but bundlers like Webpack should consider using a stripping transform (see: https://webpack.js.org/guides/production/#specify-the-environment) in production builds if they also use a minifier.