React-beautiful-dnd: Typescript in-repo

Created on 16 Feb 2018  路  7Comments  路  Source: atlassian/react-beautiful-dnd

Bug or feature request?

feature request

Expected behavior

typescript definitions can be easily maintained

Actual behavior

Webstorm goes crazy trying to index thousands of directories and I cannot even PR updates to the definitions for this library to DefinitelyTyped.

TL;DR

DefinitelyTyped is a good idea, but the organization of it makes it more difficult than just keeping the definitions here.

If I PR updated typescript definitions and tests directly to this repo, will you accept them? It is dramatically simpler to maintain per-repo.

engineering health 馃洜

Most helpful comment

I removed them from the SkateJS project because they were, in fact, a burden to maintain. Even though someone was contributing them, and maintaining them, it created a maintenance burden that, in the end, I wasn't willing to take on.

Maintenance

Firstly, if anything in the main codebase changed, then the types would have to change. You can catch this with tests. However, the tests also need to be maintained. These tests are non trivial and must be designed to cover each definition with their API point. This was costly to myself because what spare time I have to work on the project, I want to spend actually building stuff rather than doing busy work.

I think it's worth noting here that libraries can be used without types, and should not be penalised for not having them. Types are nice to have, for sure, but I don't think it's pragmatic to choose a library solely based on whether or not they have types for you. If a type system blocks you from progressing because dependency doesn't have compatible types, that doesn't feel scalable. This is, of course, my opinion, but contributed greatly to my decision to remove them.

The second part to maintenance, would be contributions. I think it's perfectly acceptable for someone to have to learn what your codebase is written in (Flow / TS / JS). For everything they have to do on top of cut code on the main path (and write tests of course) adds cognitive overhead. There's two things that can happen here:

  1. You make them write types for a separate superset (including tests).
  2. You let them get by, but then you (or someone else) has to write them for that PR.

For the first one, this is yet another barrier to entry (even if they're proficient with the tech). For the second one, this is yet another thing you have to do before you can get a release out the door.

The future

Right now we have two supersets: TS and Flow. What happens when the next greatest thing comes out?

I'm hoping something like this happens:

  • Types are spec'd into the ECMA standard as opt-in (no other way to do it for a non-typed lang).
  • Someone releases a type converter that generates types for all supersets from a single one, even if they have to be a little generic.

DefinitelyTyped

I agree that maintaining things in the DefinitelyTyped repo can be more difficult for the one maintaining them, but I think think it's the best place for it for several reasons.

  1. It's tagline is literally this "The repository for high quality TypeScript type definitions." As far as TypeScript is concerned, it seems to be the canonical way to represent types.
  2. Definitions can be maintained async and doesn't block releases to the main project. While types may fall behind, they shouldn't prohibit the usage of the library. They're a superset, and as such, should enhance the usage of a library, not make it worse.
  3. A slow / poorly-optimised IDE isn't a compelling reason to organise code a certain way.

Conclusion

While we can empathise with a certain amount of pain regarding the maintenance, we believe the benefit of maintaining them co-located with the source is outweighed by the overall burden and ask that you respect our choice not to maintain them here, at least for the foreseeable future. Thanks :)

All 7 comments

At this stage we will not be storing typescript definition files in the repo itself. We are already typing the system with flow and needing to maintain more type definitions will slow us down too much.

Unless the project code itself is being validated against the definition files then having them in the repo is just noise at best, and incorrect at worst.

Perhaps a better time to evaluate this would be in a few months. For now DefinitelyTyped is the path we'll go down

BTW you can (and I expected to) run tests against the TS definitions as part of the normal test suite. Since the definitions are already created and are very similar to the flow definitions, the incremental burden is low.

Since tests would be run here, it would ensure that the two day collosal waste of my time #332 would never occur again.

Instead you are opting to leave them at definitely typed, which most certainly means they will fall behind again, and someone else will likely once again go down the wrong rabbit hole.

Since I'm offering a better solution and I'm willing to maintain, please reconsider.

@treshugart I know you have had a similar setup before - what are your thoughts? Also any thoughts @lukebatchelor?

I removed them from the SkateJS project because they were, in fact, a burden to maintain. Even though someone was contributing them, and maintaining them, it created a maintenance burden that, in the end, I wasn't willing to take on.

Maintenance

Firstly, if anything in the main codebase changed, then the types would have to change. You can catch this with tests. However, the tests also need to be maintained. These tests are non trivial and must be designed to cover each definition with their API point. This was costly to myself because what spare time I have to work on the project, I want to spend actually building stuff rather than doing busy work.

I think it's worth noting here that libraries can be used without types, and should not be penalised for not having them. Types are nice to have, for sure, but I don't think it's pragmatic to choose a library solely based on whether or not they have types for you. If a type system blocks you from progressing because dependency doesn't have compatible types, that doesn't feel scalable. This is, of course, my opinion, but contributed greatly to my decision to remove them.

The second part to maintenance, would be contributions. I think it's perfectly acceptable for someone to have to learn what your codebase is written in (Flow / TS / JS). For everything they have to do on top of cut code on the main path (and write tests of course) adds cognitive overhead. There's two things that can happen here:

  1. You make them write types for a separate superset (including tests).
  2. You let them get by, but then you (or someone else) has to write them for that PR.

For the first one, this is yet another barrier to entry (even if they're proficient with the tech). For the second one, this is yet another thing you have to do before you can get a release out the door.

The future

Right now we have two supersets: TS and Flow. What happens when the next greatest thing comes out?

I'm hoping something like this happens:

  • Types are spec'd into the ECMA standard as opt-in (no other way to do it for a non-typed lang).
  • Someone releases a type converter that generates types for all supersets from a single one, even if they have to be a little generic.

DefinitelyTyped

I agree that maintaining things in the DefinitelyTyped repo can be more difficult for the one maintaining them, but I think think it's the best place for it for several reasons.

  1. It's tagline is literally this "The repository for high quality TypeScript type definitions." As far as TypeScript is concerned, it seems to be the canonical way to represent types.
  2. Definitions can be maintained async and doesn't block releases to the main project. While types may fall behind, they shouldn't prohibit the usage of the library. They're a superset, and as such, should enhance the usage of a library, not make it worse.
  3. A slow / poorly-optimised IDE isn't a compelling reason to organise code a certain way.

Conclusion

While we can empathise with a certain amount of pain regarding the maintenance, we believe the benefit of maintaining them co-located with the source is outweighed by the overall burden and ask that you respect our choice not to maintain them here, at least for the foreseeable future. Thanks :)

I agree that maintaining things in the DefinitelyTyped repo can be more difficult for the one >maintaining them, but I think think it's the best place for it for several reasons.

As a new user of react-beautiful-dnd I find Flow types to add a nearly show stoping burden. Getting examples to work independently of the repo code, for example (without setting up Flow support).

Having left Java for Javascript a couple of years ago, I very much appreciate the relative simplicity JS sans types provides. I really haven't encountered issues that would have been prevented by enforced typing.

That's just my opinion, but having worked for years with Java and strict typing compared to JS with (very) loose typing I'm in favor of the latter.

You should be able to use all of the code without needing to set up any flow. We strip out all of the flowtypes for all of our builds so you should not need it. Our internal examples use it - but once #343 lands you will have many more simple examples to play with

Flow is definitely not required to use this project. It is extra safety for those who want it

@alexreardon Yeah, it was the examples primarily. I've copied the simple codesandbox example and am building from there. My use case goes beyond the multiple-vertical example (think file manager), built on a Next.JS foundation.

I'm getting up to speed and it going well so far. #343 looks awesome!

Thanks for the response.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

FutureKode picture FutureKode  路  3Comments

lukyth picture lukyth  路  3Comments

crapthings picture crapthings  路  3Comments

shalkam picture shalkam  路  3Comments

h182032 picture h182032  路  3Comments