fix: TypeScript support #268
Conversation
| @@ -2,6 +2,7 @@ | |||
| "name": "react-strict-dom", | |||
| "version": "0.0.34", | |||
| "description": "React Strict DOM", | |||
| "types": "./dist/dom/index.d.ts", | |||
There was a problem hiding this comment.
I'm surprised TS doesn't pick up the d.ts file next to the module entry, like flow does.
The types for web and native are slightly different. So how would TS users get the correct types in React Native?
There was a problem hiding this comment.
Types should be defined in exports, using the types field does not make sense. But I am not sure if TypeScript can handle different types for web and native - maybe when using 2 different tsconfigs
There was a problem hiding this comment.
Typescript often does pick up .d.ts files next to the JS files like Flow does. However, there's a bunch of tooling other than the Typescript Language Server that expects the types field in the package.json.
Additionally, RSD should have the same public types across web and native so hopefully that isn't a problem?
There was a problem hiding this comment.
Nope, the exported API is not the same across platforms.
There was a problem hiding this comment.
Perhaps we can create some kind of union type of the two and make that available for the types key?
I would also like to understand from @nandorojo what issues you're running into today. Typescript does resolve types from the .d.ts files that live alongside the .js files in my own testing.
There was a problem hiding this comment.
@nandorojo @efoken See if #269 works for TS tooling integration
There was a problem hiding this comment.
This should work. However, there should be a main single export that merges the types to let consumers implement both DOM and native.
Perhaps we can create some kind of union type of the two and make that available for the types key?
We technically don’t want a union since this would only expose types for one or the other. We should have a way to merge two different types. It shouldn’t be too hard, I could help with this.
There was a problem hiding this comment.
We already successfully use Flow this way internally. So I don't think the types should be merged. I don't want people trying to use non-existent APIs on web and not getting type errors. The extra native API is only to be used in *.native.js files
There was a problem hiding this comment.
#269 looks good 👍 There should be no need for merged types
Explicitly add 'types' to each conditional package export. Close #268
|
Closing in favor of #269 |
See #25