-
Notifications
You must be signed in to change notification settings - Fork 958
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
Consider re-exporting @types/geojson types #2769
Comments
Thanks for your comments @ijxy. Have clarified in the README that @types/geojson needs to be installed separately: https://github.com/Turfjs/turf/blob/master/README.md#typescript |
It is still a terrible DX though, which is why I am asking you to consider changing your stance. It costs Turf nothing to re-export the types, but it costs all users time and energy figuring out that the types aren't re-exported (people typically consult a README after they get stuck, not before). Again, it's also a matter of encapsulation, as in the example I provided above. I have seen this many times in other corners of the web, if you do not export the types then you're essentially encouraging code such as the following: import { toPoint } from "@turf/helpers";
export type Point = ReturnType<typeof toPoint>; In a large codebase, it is unreasonable to expect every dev to come and read the README for every package. So instead, you strive to make it easy to understand what is happening. This is obvious to all: import { toPoint, Point } from "@turf/helpers"; This is not: import { toPoint } from "@turf/helpers";
import type { Point } from "totally-different-package-that-isn't-in-our-package.json-file" The fact that Appreciate the quick response, by the way, and respect your decision either way. Just wanted to make a strong case for it as I believe it is net beneficial to all Turf users by reducing mental overhead. |
Can certainly see it from your point of view. The situation is weird in a lot of ways. It being a "spec only" typedef package is unusual for a start. Ultimately, the types need to be imported from somewhere. To avoid adding unneeded coupling and possible version mismatches, it makes the most sense for Turf to refer to an external, definitive resource. From there happy to take feedback on ways we can make it less of a learning curve for devs e.g. documentation.
I know right? It's bonkers. Fwiw, Turf used to re-export the types and it was a pretty popular decision to remove that (#1658). Not saying you're wrong to have a different opinion. Just that we have to make a choice sometimes based on a range of considerations, and do our best to make up the gap in other ways. Appreciate your passion for the topic and for making the argument. |
The linked issue, as I read it, is about replacing custom type definitions with
I agree! The Turf APIs are coupled to these types–they return instances of those types in many cases. Exporting the types would not increase the coupling at all, it would just make it easier to use the types of the objects that Turf allows you to create. Edit: Also, I note that even you were caught out by the import being from |
Has something changed? There is no mention of Typescript in the readme. |
Hi @ir-fuel. The readme has since been refactored. A more up to date link would be https://turfjs.org/docs/getting-started#typescript |
@smallsaucepan I still don't understand the reluctance to re-export the types. Re-exporting the types would cost literally nothing in terms of maintenance (because you're not defining the types, just exporting them) or bundle size (types don't exist after transpilation). It would take a few minutes to set up the exports, but this time would be recouped many times over by consumers, since... Re-exporting the types would make consuming these awesome packages easier (no additional dependency, no potential version issues, no need to understand the mess that is "geojson" and "@types/geojson"). When you need documentation to explain to devs how to import/access the types that your entire suite of packages is built around, then you've probably got it wrong. |
@types/geojson
is an explicit dependency in various packages, but, despite that the types, are not exported.If I can use
@turf/helpers
to create a thing, I should be able to rely on@turf/helpers
to give me the type of the thing I create.Currently, the typical consumer journey probably goes like this:
@turf/*
packagesFor the love of code, please just make it easy to do the "RightThing". Re-export the types!
The text was updated successfully, but these errors were encountered: