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

buildFederatedSchema breaks when minified #3335

Closed
4 tasks
dncrews opened this issue Sep 24, 2019 · 4 comments · Fixed by apollographql/apollo-tooling#1551 or #3387
Closed
4 tasks

buildFederatedSchema breaks when minified #3335

dncrews opened this issue Sep 24, 2019 · 4 comments · Fixed by apollographql/apollo-tooling#1551 or #3387
Assignees
Labels
🪲 bug 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository. 🚀 shipped
Milestone

Comments

@dncrews
Copy link

dncrews commented Sep 24, 2019

There seems to be an issue when using buildFederatedSchema while minified.
Per the documentation, when you're using federation, you are expected to use
the extend keyword on types that will be merged with other schemas:

extend type Query {
  ping: String!
}

This syntax breaks when minified, causing the error:

Error: Cannot extend type "Query" because it is not defined.

Steps to replicate:

  • Clone this repository
  • run yarn or npm install to install the depedencies
  • run yarn start or npm start to run the non-minified code. You will see the output line "Built"
  • run yarn bug or npm run bug to minify and run the minified code. The expected result would be to see "Built" again. The actual result is that it errors out with the aforementioned error.
@dncrews
Copy link
Author

dncrews commented Sep 24, 2019

You may be tempted to ask:

but why would you minify your Apollo Server?

to which I would reply

serverless performance

@abernix
Copy link
Member

abernix commented Sep 25, 2019

Can you attach a debugger and investigate what is different at runtime between the two scenarios?

@abernix abernix added 🪲 bug 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository. 👩‍🚀 federation labels Sep 25, 2019
@abernix
Copy link
Member

abernix commented Sep 25, 2019

Ah, okay: In case it saves you from the debugger, this is due to the way that we're disabling particular validation rules via leveraging Function.prototype.name rather than object equality:

https://github.com/apollographql/apollo-tooling/blob/933c3d44b9e5fb7e3c0b462f60ce5b2d7875b15d/packages/apollo-graphql/src/schema/buildSchemaFromSDL.ts#L35-L43

Of course, the Function.prototype.name is not stable during minification, and thus the rules which should be by-passed are still being enforced.

abernix added a commit that referenced this issue Sep 25, 2019
…ion.

The previous technique for deciding which validation rules from
`specifiedSDLRules` to by-pass during
composition was leveraging a string-comparison against
`Function.prototype.name`.

As shown in #3335, that
technique breaks down under minification, when function names are often
munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference
equality with greater success, since those will not be affected by
minification.

While the actual bug in #3335 was _not_ in this code, this code poses the
same hazard and would likely be affected as well (eventually, at least).
abernix added a commit that referenced this issue Sep 25, 2019
…ion.

The previous technique for deciding which validation rules from
`specifiedSDLRules` to by-pass during
composition was leveraging a string-comparison against
`Function.prototype.name`.

As shown in #3335, that
technique breaks down under minification, when function names are often
munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference
equality with greater success, since those will not be affected by
minification.

While the actual bug in #3335 was _not_ in this code, this code poses the
same hazard and would likely be affected as well (eventually, at least).
abernix added a commit that referenced this issue Sep 25, 2019
…ion.

The previous technique for deciding which validation rules from
`specifiedSDLRules` to by-pass during
composition was leveraging a string-comparison against
`Function.prototype.name`.

As shown in #3335, that
technique breaks down under minification, when function names are often
munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference
equality with greater success, since those will not be affected by
minification.

While the actual bug in #3335 was _not_ in this code, this code poses the
same hazard and would likely be affected as well (eventually, at least).
abernix added a commit that referenced this issue Sep 27, 2019
…ion. (#3338)

* Use reference-equality when omitting validation rules during composition.

The previous technique for deciding which validation rules from
`specifiedSDLRules` to by-pass during
composition was leveraging a string-comparison against
`Function.prototype.name`.

As shown in #3335, that
technique breaks down under minification, when function names are often
munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference
equality with greater success, since those will not be affected by
minification.

While the actual bug in #3335 was _not_ in this code, this code poses the
same hazard and would likely be affected as well (eventually, at least).

* Add changelog entry
abernix added a commit to apollographql/apollo-tooling that referenced this issue Sep 30, 2019
Similar in spirit to apollographql/apollo-server#3338.

The technique previously used for removing rules from the standard
`specifiedRules` was leveraging a check on `Function.prototype.name`, rather
than doing direct object equality.  While that does generally work, thanks
to the more recent standardization of `Function.prototype.name`, it still
breaks down under some of the more aggressive minification techniques since
a function's name is not guaranteed to remain the same.

Fixes: apollographql/apollo-server#3335
trevor-scheer pushed a commit to apollographql/apollo-tooling that referenced this issue Oct 2, 2019
…1551)

Use object equality when filtering rules in `buildSchemaFromSDL`

Similar in spirit to apollographql/apollo-server#3338.

The technique previously used for removing rules from the standard
`specifiedRules` was leveraging a check on `Function.prototype.name`, rather
than doing direct object equality.  While that does generally work, thanks
to the more recent standardization of `Function.prototype.name`, it still
breaks down under some of the more aggressive minification techniques since
a function's name is not guaranteed to remain the same.

Fixes: apollographql/apollo-server#3335
@abernix abernix reopened this Oct 2, 2019
@abernix
Copy link
Member

abernix commented Oct 2, 2019

Once we release the (landed/merged outside this repo) fix from apollographql/apollo-tooling#1551 and update apollo-graphql within Apollo Server, this can be closed.

@abernix abernix added this to the Release 2.x milestone Oct 2, 2019
trevor-scheer added a commit that referenced this issue Oct 7, 2019
This commit updates the apollo-graphql package to the latest
version. This includes a fix for a bug that currently breaks
minification. For reference, see:
apollographql/apollo-tooling#1551

Fixes #3335
abernix pushed a commit that referenced this issue Oct 8, 2019
This commit updates the apollo-graphql package to the latest
version. This includes a fix for a bug that currently breaks
minification. For reference, see:
apollographql/apollo-tooling#1551

Fixes #3335
@abernix abernix modified the milestones: Release 2.x, Release 2.9.4 Oct 8, 2019
abernix added a commit to apollographql/federation that referenced this issue Sep 4, 2020
…ion. (apollographql/apollo-server#3338)

* Use reference-equality when omitting validation rules during composition.

The previous technique for deciding which validation rules from
`specifiedSDLRules` to by-pass during
composition was leveraging a string-comparison against
`Function.prototype.name`.

As shown in apollographql/apollo-server#3335, that
technique breaks down under minification, when function names are often
munged to shorter alternatives.

As an alternative, we can import the rules and check them for reference
equality with greater success, since those will not be affected by
minification.

While the actual bug in apollographql/apollo-server#3335 was _not_ in this code, this code poses the
same hazard and would likely be affected as well (eventually, at least).

* Add changelog entry

Apollo-Orig-Commit-AS: apollographql/apollo-server@0bad61d
abernix pushed a commit to apollographql/federation that referenced this issue Sep 4, 2020
…3387)

This commit updates the apollo-graphql package to the latest
version. This includes a fix for a bug that currently breaks
minification. For reference, see:
apollographql/apollo-tooling#1551

Fixes apollographql/apollo-server#3335
Apollo-Orig-Commit-AS: apollographql/apollo-server@6a4b681
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🪲 bug 🌹 has-reproduction ❤ Has a reproduction in Glitch, CodeSandbox or Git repository. 🚀 shipped
Projects
None yet
3 participants