Skip to content

definition: replace object spread with explicit assignments#2089

Merged
IvanGoncharov merged 1 commit intographql:masterfrom
IvanGoncharov:master
Aug 14, 2019
Merged

definition: replace object spread with explicit assignments#2089
IvanGoncharov merged 1 commit intographql:masterfrom
IvanGoncharov:master

Conversation

@IvanGoncharov
Copy link
Copy Markdown
Member

@IvanGoncharov IvanGoncharov commented Aug 14, 2019

In preparation for #1527

@IvanGoncharov IvanGoncharov added the PR: polish 💅 PR doesn't change public API or any observed behaviour label Aug 14, 2019
@IvanGoncharov
Copy link
Copy Markdown
Member Author

IvanGoncharov commented Aug 14, 2019

Also, it strangely produces significant speedup (almost 50%) for buildSchema and buildClientSchema functions:
image
image

@IvanGoncharov IvanGoncharov force-pushed the master branch 2 times, most recently from 70de26a to e04d92b Compare August 14, 2019 14:35
@IvanGoncharov IvanGoncharov merged commit 0105d92 into graphql:master Aug 14, 2019
IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Aug 15, 2019
@IvanGoncharov
definitions: make constructed fields non-optional
df05745 
Reflects changes in graphql#2089
@IvanGoncharov IvanGoncharov mentioned this pull request Aug 15, 2019
Merged
IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Aug 15, 2019
@IvanGoncharov
definitions: make constructed fields non-optional
f498b7b 
Reflects changes in graphql#2089
IvanGoncharov added a commit that referenced this pull request Aug 15, 2019
@IvanGoncharov
definitions: make constructed fields non-optional (#2091)
901c4e7 
Reflects changes in #2089
@taion
Copy link
Copy Markdown
Contributor

taion commented Aug 23, 2019

@IvanGoncharov

This is sort of a breaking change in practice. Because of the older spread-through, it was possible to define arbitrary other properties on fields, e.g. for https://github.com/4Catalyzer/graphql-validation-complexity#usage. This now no longer works.

It's true that this was undocumented behavior, but I also don't think it was quite that rare in practice.

@IvanGoncharov
Copy link
Copy Markdown
Member Author

IvanGoncharov commented Aug 23, 2019

@taion Can you please open an issue?
Can you switch to extensions mechanism added in #2097?

but I also don't think it was quite that rare in practice.

If there are devs stuck with this hack and can't switch to extensions I don't have a problem to temporary revert it until 15.0.0.
But it is 100% happening in 15.0.0 and everyone will need to switch to extensions anyway.

@taion taion mentioned this pull request Aug 23, 2019
Closed
IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Aug 23, 2019
@IvanGoncharov
Copy non-standard properites from field's config
0c60413 
Fixes graphql#2114
Partly reverts graphql#2089
@IvanGoncharov IvanGoncharov mentioned this pull request Aug 23, 2019
Merged
IvanGoncharov added a commit that referenced this pull request Aug 23, 2019
@IvanGoncharov
Copy non-standard properites from field's config (#2116)
3104e1f 
Fixes #2114
Partly reverts #2089
This was referenced Aug 31, 2019
Merged
Merged
Merged
Merged
Merged
This was referenced Oct 26, 2019
Open
Open
@snyk-bot snyk-bot mentioned this pull request Dec 5, 2019
Open
@snyk-bot snyk-bot mentioned this pull request Jan 16, 2020
Open
@snyk-bot snyk-bot mentioned this pull request Feb 12, 2020
Closed
@snyk-bot snyk-bot mentioned this pull request Jan 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

PR: polish 💅 PR doesn't change public API or any observed behaviour

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@IvanGoncharov @taion