Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Bs platform 7 #222

Merged
merged 5 commits into from
Dec 23, 2019
Merged

Bs platform 7 #222

merged 5 commits into from
Dec 23, 2019

Conversation

Gregoirevda
Copy link
Contributor

@Gregoirevda Gregoirevda commented Dec 23, 2019

Pull Request Labels

  • feature
  • blocking
  • docs

bs platform 7 support

_ =
"";

// [@bs.obj]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably delete

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as not used anymore in createApolloClient

),
);
};
// let webSocketLink = (~uri, ~reconnect=?, ~connectionParams=?, ()) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be deleted as all links should be in ApolloLinks

type webSocketLinkOptionsT = {
[@bs.optional]
reconnect: bool,
Copy link
Contributor Author

@Gregoirevda Gregoirevda Dec 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional in the new typed object

),
)
jsMutation({variables, refetchQueries, optimisticResponse})
// jsMutation(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete

.
"subscriptionData": Js.Nullable.t(Js.Json.t),
"variables": Js.Nullable.t(Js.Json.t),
subscriptionData: option(Js.Json.t),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be null & undefined: switch to Js.Nullable.t

"subscriptionData": Js.Nullable.t(Js.Json.t),
"variables": Js.Nullable.t(Js.Json.t),
subscriptionData: option(Js.Json.t),
variables: option(Js.Json.t),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be null & undefined: switch to Js.Nullable.t

@Gregoirevda
Copy link
Contributor Author

In general double check option vs Js.Nullable.t: in
JS -> Reason: null -> Some('')
Reason -> JS: Some('') -> ''

@Gregoirevda Gregoirevda merged commit f069289 into apollographql:master Dec 23, 2019
@Gregoirevda
Copy link
Contributor Author

@idkjs Thanks a lot for your work!! I added some comments to myself, not sure if always relevant, but need to look into.

@Gregoirevda
Copy link
Contributor Author

Fixes #220 #221

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant