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

Emitted types break default setup when colocating artefacts. #58

Closed
alloy opened this issue Aug 14, 2018 · 6 comments
Closed

Emitted types break default setup when colocating artefacts. #58

alloy opened this issue Aug 14, 2018 · 6 comments

Comments

@alloy
Copy link
Member

alloy commented Aug 14, 2018

As reported by @ckknight https://twitter.com/ckknight/status/1026846806930468864, it appears that when one does not enable a single artefact directory –and thus fragment references get typed as any rather than being imported from other artefacts– TS is unable to work with the types:

Type 'any' is not assignable to type 'unique symbol'.

I haven’t yet been able to find any issues on the TS issue tracker about this, so unsure right now how to proceed.

Maybe we should not emit the fragment reference typings when not using the single artefact directory feature, because those typings are moot in that case anyways?

@kastermester
Copy link
Contributor

kastermester commented Aug 14, 2018

I see now.

This does indeed seem quite weird. I think there's still value in emitting the types (just as flow does) but emit those as any as well (ie. not as a unique symbol type).

Ie. for the above linked playground we should emit something like this instead

EDIT:

The value we gain is that it allows people to use Relay with typescript without having to typecast to any when rendering the components, as the typings inside the react-relay package will now simply mark that the property value doesn't matter, but still transform the prop types.

@alloy
Copy link
Member Author

alloy commented Aug 14, 2018

Fair, we could do that too. What’s your thought for there still being value? Just as a named label that could help indicate what reference is required?

@kastermester
Copy link
Contributor

I just edited in my thoughts as you replied, realised it might not be obvious why it would be a good idea.

@alloy
Copy link
Member Author

alloy commented Aug 14, 2018

@ckknight Are you perchance interested in submitting a PR for that? The type generator receives a flag that indicates wether or not the single artefact directory feature is enabled.

Having said all that, and we absolutely should not break the default setup, I would suggest you consider enabling the single artefact directory feature after all, as it’s the only stable simple way we could come up with that would allow you to have typed fragment references without using the Haste module system.

@kastermester
Copy link
Contributor

To be honest I would not be too surprised if using the single artifact directory ends out being the way most people will use relay going forward (either that or we get some nifty way of tracking where the artifacts are being output in the future such that we can get around the issues we're circumventing by using this feature). It helps both Flow and TypeScript users get the typechecking features they deserve.

With that said - making the changes stated here makes sense, it will help us reach parity with the Flow implementation.

@alloy
Copy link
Member Author

alloy commented Aug 14, 2018

Agreed. facebook/relay#2509

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

No branches or pull requests

2 participants