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

fix(stream): allow initialValue of zero #2979

Merged
merged 2 commits into from
Apr 6, 2021

Conversation

yaacovCR
Copy link
Contributor

No description provided.

@yaacovCR
Copy link
Contributor Author

@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

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

@yaacovCR
Copy link
Contributor Author

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

@robrichard @IvanGoncharov ready for review

@robrichard
Copy link
Contributor

@yaacovCR can you add a test for this case?

@yaacovCR
Copy link
Contributor Author

Sure

@yaacovCR
Copy link
Contributor Author

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 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
Labels
stream/defer Issues/PRs related to experimental steam/defer support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants