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

Add types for database #501

Closed

Conversation

parzhitsky
Copy link
Contributor

Add type-safety and IntelliSense (editor suggestions) to database file

@AnandChowdhary
Copy link
Contributor

I don't know if this is a great idea, other projects may also rely upon the database.json file (like ours does) which this PR would break. I personally think it's much easier to consume JSON than an object in many use cases... :(

@parzhitsky
Copy link
Contributor Author

The fact that this is a breaking change is actually my biggest concern. Would you mind jotting about how exactly your project uses the json? Perhaps, it might be fixed by dynamically generating files at some point, or something like that (or something completely different 🙂).

As for ease of use, I tend to disagree, since a) with importing (or otherwise incorporating) the file you import both value (i.e., JavaScript implementation) and types in one go; b) there's no need to parse/stringify anything, as some systems require; c) third-party contributions to that file are validated earlier; and so on.

@AnandChowdhary
Copy link
Contributor

We have a deno.land/x mirror that fetches each package from database.json here: https://github.com/denorg/x.

It's trivial for me to change the download script to support the package.ts file, but my general idea is that JSON is more universal, better-supported in other contexts (non-JavaScript scripts, for example), and better-understood by computers that don't use IntelliSense.

@AnandChowdhary
Copy link
Contributor

But I think this might become moot as Deno's package registry evolves and doesn't have a database.json anymore: #406

@parzhitsky
Copy link
Contributor Author

Ooo, I’m glad they’re thinking this through! The whole approach with npm-ish global scope of unique names updated manually through pull requests seamed flawed from the start.

Thanks for the link!

@lucacasonato
Copy link
Member

I think JSON is better suited here and there are very valid usecases for importing this json file externally. I think keeping it as JSON makes more sense because it is more accessible. We have tests to catch style errors in the database anyway.

@ry
Copy link
Member

ry commented May 16, 2020

I appreciate the patch, but JSON for data seems more versatile to me.

@ry ry closed this May 16, 2020
@parzhitsky parzhitsky deleted the feature/types-for-database branch May 17, 2020 09:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants