Skip to content

fix(stream): allow initialValue of zero#2979

Merged
robrichard merged 2 commits intographql:defer-streamfrom
yaacovCR:zero-initial-count
Apr 6, 2021
Merged

fix(stream): allow initialValue of zero#2979
robrichard merged 2 commits intographql:defer-streamfrom
yaacovCR:zero-initial-count

Conversation

@yaacovCR
Copy link
Contributor

@yaacovCR yaacovCR commented Mar 22, 2021

No description provided.

@yaacovCR yaacovCR force-pushed the zero-initial-count branch from e0e3cad to 2caec2d Compare March 22, 2021 16:45
@yaacovCR yaacovCR force-pushed the zero-initial-count branch from 6d08565 to 38f678c Compare March 22, 2021 17:19
@yaacovCR
Copy link
Contributor Author

yaacovCR commented Mar 22, 2021

@IvanGoncharov

I am not sure why the flow integration tests fail -- they pass if we bump the flow version used in integration tests to 0.142 to match the base repo.

On the other hand, if I downgrade the base repo to 0.135, it triggers the following seemingly unrelated errors:

> [email protected] check C:\Users\Adina\Documents\GitHub\graphql-js
> flow check

Error ---------------------------------------------------------------------------------------- flow-typed/core.js:131:28

Please add `...` to the end of the list of properties to express an inexact object type. [implicit-inexact-object]

   131|     static entries<T>(obj: { [key: string]: T, __proto__: null }): Array<[string, T]>; // graphql-js HACK
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Error ---------------------------------------------------------------------------------------- flow-typed/core.js:131:28

Please write this object type as explicitly exact (use `{|` and `|}` instead of `{` and `}`) or as explicitly inexact
(add `...` to the end of the list of properties). [ambiguous-object-type]

   131|     static entries<T>(obj: { [key: string]: T, __proto__: null }): Array<[string, T]>; // graphql-js HACK
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Error ---------------------------------------------------------------------------------------- flow-typed/core.js:223:27

Please add `...` to the end of the list of properties to express an inexact object type. [implicit-inexact-object]

   223|     static values<T>(obj: { [key: string]: T, __proto__: null }): Array<T>; // graphql-js HACK
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Mar 22, 2021

huh. and now integration tests are passing again. wierd.

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Mar 22, 2021

I didn't bother adding an additional test -- the existing code was not allowing an asynciterator to stream initialValues=0, was always acting as if initialValues was at least 1.

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Mar 22, 2021

@robrichard @IvanGoncharov ready for review

@yaacovCR yaacovCR mentioned this pull request Mar 23, 2021
Closed
@robrichard
Copy link
Contributor

robrichard commented Mar 23, 2021

@yaacovCR can you add a test for this case?

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Mar 24, 2021

Sure

@yaacovCR
Copy link
Contributor Author

yaacovCR commented Mar 24, 2021

Done

@IvanGoncharov IvanGoncharov added the stream/defer Issues/PRs related to experimental steam/defer support label Mar 31, 2021
@robrichard robrichard merged commit 1012e3e into graphql:defer-stream Apr 6, 2021
yaacovCR added a commit to ardatan/graphql-tools that referenced this pull request May 1, 2021
@yaacovCR
apply graphql-js patches
937f5c6 
graphql/graphql-js#2980
graphql/graphql-js#2979
graphql/graphql-js#2975
@yaacovCR yaacovCR deleted the zero-initial-count branch October 25, 2021 18:06
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

stream/defer Issues/PRs related to experimental steam/defer support

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yaacovCR @robrichard @IvanGoncharov

Comments