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

TypeScript defs for Helper Functions with Hooks #72

Closed
eddiewang opened this issue Aug 5, 2019 · 10 comments · Fixed by #73
Closed

TypeScript defs for Helper Functions with Hooks #72

eddiewang opened this issue Aug 5, 2019 · 10 comments · Fixed by #73

Comments

@eddiewang
Copy link

The README shows a declarative approach to writing hook-based react-async resolvers. However, if implemented with Typescript, the following error appears:

const Rejected: <T>(props: {
    children?: AsyncChildren<T>;
    persist?: boolean;
}) => JSX.Element
Type '{ children: (error: AsyncState<unknown>) => Element; state: AsyncState<StandardResp<CoinProfileResp>>; }' is not assignable to type 'IntrinsicAttributes & { children?: AsyncChildren<unknown>; persist?: boolean; }'.
  Property 'state' does not exist on type 'IntrinsicAttributes & { children?: AsyncChildren<unknown>; persist?: boolean; }'.

Going into the definitions, it seems like state isn't considered a prop that is passed through. The same goes for Pending and Fufilled components.

@eddiewang
Copy link
Author

eddiewang commented Aug 6, 2019

I did some more research. I had previously imported the component like so:

import { AsyncState, Async } from 'react-async'
//...
const { Fulfilled, Pending, Rejected } = Async

It seems like the components on the Async component are different than the helper.js components. However when I try to import them like in the docs:

import { AsyncState, Fulfilled, Pending, Rejected } from 'react-async'

I got a TS error:

Module '"../../../../../../../../Users/eddiewang/js/src/github.com/test/sentinel-react-native/node_modules/react-async/dist-types"' has no exported member 'Rejected'. Did you mean 'Reject'?ts(2724)

All three components (Fulfilled, Pending, Rejected) show as unavl in the typescript defs. Also, my app does not compile, with this error:

Warning: Failed prop type: Invalid prop `state.error` of type `Response` supplied to `Pending`, expected instance of `Error`.
react-native/node_modules/react-async/dist-types"' has no exported member 'Fulfilled'. Did you mean 'Fulfill'?

@ghengeveld
Copy link
Member

Hey, thanks for reporting. The problem is with a naming collision between the stand-alone <Fulfilled> (etc) versus <Async.Fulfilled>. When importing Fulfilled there is ambiguity if that means the stand-alone version or the static member of Async.

I have a fix for this ready, see #73

Care to review?

@eddiewang
Copy link
Author

Interesting. Your commit seems to resolve the TS error. Does TS by default export class methods as their base name? Confused as to why Async.Fulfilled gets exported to Fulfilled.

I'll test the compilation in a few hours after a meeting. Thanks for the help!

@eddiewang
Copy link
Author

@ghengeveld here's the error I'm getting now:

Warning: Failed prop type: Invalid prop `state.error` of type `Response` supplied to `IfRejected`, expected instance of `Error`.
- node_modules/expo/build/environment/muteWarnings.fx.js:26:24 in error
- node_modules/prop-types/checkPropTypes.js:20:20 in printWarning
- node_modules/prop-types/checkPropTypes.js:83:12 in checkPropTypes
- node_modules/react/cjs/react.development.js:1666:19 in validatePropTypes
- node_modules/react/cjs/react.development.js:1755:22 in createElementWithValidation
* atoms/Card.tsx:83:9 in <unknown>
- node_modules/react-native/Libraries/Renderer/oss/ReactNativeRenderer-dev.js:9473:27 in renderWithHooks
- node_modules/react-native/Libraries/Renderer/oss/ReactNativeRenderer-dev.js:11381:6 in updateFunctionComponent
- ... 21 more stack frames from framework internals

Warning: Failed prop type: Invalid prop `state.error` of type `Response` supplied to `IfFulfilled`, expected instance of `Error`.
- node_modules/expo/build/environment/muteWarnings.fx.js:26:24 in error
- node_modules/prop-types/checkPropTypes.js:20:20 in printWarning
- node_modules/prop-types/checkPropTypes.js:83:12 in checkPropTypes
- node_modules/react/cjs/react.development.js:1666:19 in validatePropTypes
- node_modules/react/cjs/react.development.js:1755:22 in createElementWithValidation
* atoms/Card.tsx:90:9 in <unknown>
- node_modules/react-native/Libraries/Renderer/oss/ReactNativeRenderer-dev.js:9473:27 in renderWithHooks
- node_modules/react-native/Libraries/Renderer/oss/ReactNativeRenderer-dev.js:11381:6 in updateFunctionComponent
- ... 21 more stack frames from framework internals

Warning: Failed prop type: Invalid prop `state.error` of type `Response` supplied to `IfPending`, expected instance of `Error`.
- node_modules/expo/build/environment/muteWarnings.fx.js:26:24 in error
- node_modules/prop-types/checkPropTypes.js:20:20 in printWarning
- node_modules/prop-types/checkPropTypes.js:83:12 in checkPropTypes
- node_modules/react/cjs/react.development.js:1666:19 in validatePropTypes
- node_modules/react/cjs/react.development.js:1755:22 in createElementWithValidation
* atoms/Card.tsx:78:9 in <unknown>
- node_modules/react-native/Libraries/Renderer/oss/ReactNativeRenderer-dev.js:9473:27 in renderWithHooks
- node_modules/react-native/Libraries/Renderer/oss/ReactNativeRenderer-dev.js:11381:6 in updateFunctionComponent
- ... 21 more stack frames from framework internals

No Typescript errors though. I'm using the useFetch hook returning an AsyncState<T> and passing that in as the state.

@eddiewang
Copy link
Author

eddiewang commented Aug 6, 2019

@ghengeveld do you have a process for linking the libraries locally? Would be willing to add that into contributing.md.

yarn link causes issues related to babel/runtime require resolve.

babel/babel-loader#149

@ghengeveld
Copy link
Member

ghengeveld commented Aug 7, 2019

Yes, it uses relative-deps for the examples and Yarn workspaces with Lerna for the packages. The general process is to run yarn bootstrap after the first install. Should be smooth sailing from there. Sometimes Yarn messes things up and you need to do yarn clean && yarn bootstrap to clear it up.

Don't use yarn link on examples, it won't work due to the way packages are built (with multiple compilation targets).

@eddiewang
Copy link
Author

Thanks, I haven't heard of relative-deps before, so I'll try that. I need to start looking for Lerna and Yarn workspaces... both are seemingly getting very common these days!

I have to say, using React Async has been an eye-opener for me. Async calls in React always felt like too much boilerplate, no matter which solution. This library is refreshingly flexible and seemingly future-proof! Kudos.

@ghengeveld
Copy link
Member

I have to say, using React Async has been an eye-opener for me. Async calls in React always felt like too much boilerplate, no matter which solution. This library is refreshingly flexible and seemingly future-proof! Kudos.

Thanks!

@ghengeveld
Copy link
Member

ghengeveld commented Aug 8, 2019

Hey, can you try out react-async@8.0.0-alpha.0? It's the pre-release version I just published containing the fix for this.

It can also be installed as react-async@next.

ghengeveld added a commit that referenced this issue Aug 9, 2019
* Fix ambiguous import issue by renaming the standalone helper components. Improved the TS type definitions.

* Update example to use standalone helpers.

* Add codemods for breaking API changes.

* ES modules don't work from a codemod URL.

* Ignore node_modules in cwd.

* Improve types for promiseFn/deferFn props.

* Bump and align all package versions.
@ghengeveld
Copy link
Member

Released in v8.0.0

Thanks for helping out!

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

Successfully merging a pull request may close this issue.

2 participants