Closed onpaws closed 4 years ago
This is the error I'm currently trying to understand:
From looking at the TokenRefreshLink
implementation, I see we extend ApolloLink:
I also see Apollo's own HttpLink also extends ApolloLink:
Why does TypeScript want me to somehow bring onError
/setOnError
properties to the TokenRefreshLink class, but not to the HttpLink class?
Feels like I must be missing something. If anyone has any insight would be much appreciated.
For context, here is the toKey
removal in @apollo/client
-- https://github.com/apollographql/apollo-client/commit/07463bcb0acd21d59f08c8213facf49e10f0b3e4 which is why we have to make a backwards incompatible change to comply with Apollo 3+
WRT the typescript error:
declare
on export in case the TS compiler is having issues locating the types for ApolloLink
. According to the docs, only the react stuff has complete typing. Checking the apollo repo, there is definitely not a .d.ts
in there.Thanks for putting this together!
I'm not sure, you done it right way. Please, take a look at this example https://github.com/apollographql/apollo-link/blob/%40apollo/link-batch%402.0.0-beta.3, and rest similar at that repo
Thanks for the pointers @Jakobo
You can try declare on export in case the TS compiler is having issues locating the types for ApolloLink. According to the docs, only the react stuff has complete typing. Checking the apollo repo, there is definitely not a .d.ts in there.
Dug into this a bit & it seems .d.ts
artifacts are generated as part of a CircleCI TypeScript build, and they don't live inside the source repo.
Here's what I see in a project's node_modules
that depends on @apollo/client
:
I tried declare
, thanks! Trying to learn the details on how that works. For now unfortunately TS build throws "An implementation cannot be declared in ambient contexts.", womp womp.
You can explicitly add onError/setOnError (I wouldn't recommend this, but it'd reveal if the issue is TS or something in how we've declared the class)
Right, since base class ApolloLink
already defines these properties, am finding it a bit weird why TypeScript can't see them on TokenRefreshLink
. If anything hopefully I learn something about the way TypeScript class inheritance works by the end of this... :)
Going to keep pushing.
I'm not sure, you done it right way. Please, take a look at this example https://github.com/apollographql/apollo-link/blob/%40apollo/link-batch%402.0.0-beta.3, and rest similar at that repo
Thanks @newsiberian! For me this URL works: https://github.com/apollographql/apollo-link/tree/%40apollo/link-error%402.0.0-beta.3
I will take a look at the @apollo/link-batch
.
https://github.com/apollographql/apollo-link/blob/%40apollo/link-error%402.0.0-beta.3/packages/apollo-link-batch/src/batchLink.ts
I believe by its nature a batch link it has to be a stateful link, similarly to TokenRefreshLink
.
I'm not a TypeScript expert but hoping to improve my knowledge as I go through this. Thanks
I think this may be working now 🥳 @newsiberian I carefully followed the example you shared, and was able to get TypeScript happy again. I see the Apollo team used a namespace to wrap the Options object for the link constructor, and after I did the same here TypeScript compiled fine.
In my app which is using Apollo 3.0, I'm using this and the refresh token is getting automatically requested in the desired way.
So that's good news for me, but since this is a library it would be nice to hear if this is working for other people too. Feedback appreciated!
@Jakobo if you have time you could maybe try this branch in your Apollo 3.0 project? I think a
git clone git@github.com:onpaws/apollo-link-token-refresh.git
git checkout apollo-3.0-no-subscriptions
yarn install; yarn build; yarn link
and then over in your project dir you could try
yarn link apollo-link-token-refresh
should work.
FYI, normally I'd update tests but I noticed yarn test
doesn't appear to be working on the master
branch. Do they work for you guys?
Change lgtm. Checked it out, built it successfully, and got the linked code working.
Not sure about the test failures though. Thanks @onpaws for sticking with this!
Good to hear that it works for you too @Jakobo, thanks for your feedback!
Tests was broken from the beginning as long as I remember
Tests was broken from the beginning as long as I remember
OK, no problem. Tests should be working now. Went ahead and updated deps while I was there.
$ yarn jest
yarn run v1.22.4
PASS src/__tests__/tokenRefreshLink.ts
TokenRefreshLink
✓ should construct when required arguments are passed to constructor (2ms)
✓ should throw an exception if link is the last in composed chain (81ms)
✓ passes forward on (1ms)
✓ should throw an exception if it was thrown inside the promise
OperationQueuing
✓ should construct (1ms)
✓ should be able to add to the queue
Test Suites: 1 passed, 1 total
Tests: 6 passed, 6 total
Snapshots: 0 total
Time: 1.195s, estimated 2s
Ran all test suites.
✨ Done in 2.16s.
If I understand right, one thing that's missing from the tests is that we're not actually testing whether the token fetch succeeded, just whether there was an error while fetching.
I will need to learn how to mock server responses in Jest properly, but if I have time this week I will try to add that as another test case.
I can confirm this pr works as well, would be great to see it integrated
Thanks for the feedback @luhagel good to hear 🎉
Looks like it's working correctly for me too, thank you!
I'm guessing this will be merged once v3 is out beta?
@onpaws , thanks for attempting this. Sincerely appreciated.
I have just cloned your branch and have started trying it out. Here are the initial results.
I can confirm that it is working.
I was seeing various typescript compilation issues but that was due to my environment having mismatched installed versions of @apollo/client
. Once I cleaned them up to have a consistent set of installed @apollo/client
then all was good.
Great, glad to hear it's working for you guys!
Thank you, @onpaws
I solved ApolloLink problem as follows.
must Add @apollo/client, and change apollo-boost to @apollo/client
import React from "react";
import ReactDOM from "react-dom";
import { ApolloProvider } from "@apollo/react-hooks";
import { getAccessToken, setAccessToken } from "./accessToken";
import { App } from "./App";
import {
ApolloClient,
InMemoryCache,
Observable,
ApolloLink,
HttpLink,
} from "@apollo/client";
import { onError } from "apollo-link-error";
import { TokenRefreshLink } from "apollo-link-token-refresh";
import jwtDecode, { JwtPayload } from "jwt-decode";
const cache = new InMemoryCache({});
const requestLink = new ApolloLink(
(operation, forward) =>
new Observable((observer) => {
let handle: any;
Promise.resolve(operation)
.then((operation) => {
const accessToken = getAccessToken();
if (accessToken) {
operation.setContext({
headers: {
authorization: `bearer ${accessToken}`,
},
});
}
})
.then(() => {
handle = forward(operation).subscribe({
next: observer.next.bind(observer),
error: observer.error.bind(observer),
complete: observer.complete.bind(observer),
});
})
.catch(observer.error.bind(observer));
return () => {
if (handle) handle.unsubscribe();
};
})
);
const client = new ApolloClient({
link: ApolloLink.from([
new TokenRefreshLink({
accessTokenField: "accessToken",
isTokenValidOrUndefined: () => {
const token = getAccessToken();
if (!token) {
return true;
}
try {
const { exp } = jwtDecode<JwtPayload>(token);
if (Date.now() >= exp! * 1000) {
return false;
} else {
return true;
}
} catch {
return false;
}
},
fetchAccessToken: () => {
return fetch("http://localhost:4000/refresh_token", {
method: "POST",
credentials: "include",
});
},
handleFetch: (accessToken) => {
setAccessToken(accessToken);
},
handleError: (err) => {
console.warn("Your refresh token is invalid. Try to relogin");
console.error(err);
},
}),
onError(({ graphQLErrors, networkError }) => {
console.log(graphQLErrors);
console.log(networkError);
}) as any,
requestLink,
new HttpLink({
uri: "http://localhost:4000/graphql",
credentials: "include",
}),
]),
cache,
});
ReactDOM.render(
<ApolloProvider client={client}>
<App />
</ApolloProvider>,
document.getElementById("root")
);
This PR is intended to make the minimum changes necessary to get Apollo 3.0 support working. (I'm abandoning the previous PR because apparently generating keys may not be reliable enough.)
Warning: I'm not an Apollo or TypeScript super expert, just another dev on GitHub. Credit for this approach goes to @jakobo per his comment.
TODO:
globals
updated for 3.0yarn build
(rollup compile works)peerDependencies
Hopefully @icco can speak to this.° See next comment - TypeScript is complaining about missing properties.