Skip to content

Conversation

@toverux
Copy link
Contributor

@toverux toverux commented Aug 9, 2018

Hello @nodkz,

After graphql-compose/graphql-compose#82, I'm happy to propose definition files for graphql-compose-mongoose aswell.
Again, it's a "rapid" port from the flow version.

I've copied all the configuration from graphql-compose so you've got the same commands, structure and configuration files.

I've typed all of the API except the discriminators part. It's a bit complicated and I don't want to create too much files. Also, it's probably less used than the classic API.

For the part that has been typed, I've typed all exported symbols at module's level. That means not only what's just accessible on lib/index.d.ts, but all internal contents of the library. This is still useful when you have a semi-public API on submodules, or to maintain individual files by just copying the structure of the .js counterpart without wondering each time if it's exported by lib/index.d.ts.
I think it's the same in graphql-compose, but I don't really remember. Should we keep it like that?

I've also left a few @ts-todo-marked comments where porting the Flow version was not possible/difficult:

image

It's been tested in my latest project.

I think it may also interest @mernxl in #107 (comment) even id I didn't typed his feature 😄
I've made composeWithMongooseDiscriminators useable though, but the rest of this API is not available.
To answer his question about generated definitions, no, .d.ts generators are not really useable, runtime-based ones provide wrong or low-quality files, and Flow to TS converters are still experimental and will probably stay so.


I want to thank you again for all the work you've done on these libraries. My team uses compose libraries on all new projects and we're really happy with them. That's hundreds of hours or LoCs saved.
We're using the mongoose plugin from the beginning but I've never taken the time to type it like the main one, and we were still using an auto-generated stub of very low quality. I would be happy to benefit from real typings!

@nodkz nodkz merged commit 723ae7c into graphql-compose:master Aug 9, 2018
@nodkz
Copy link
Member

nodkz commented Aug 9, 2018

Wow! Thank you for this! 👍

@nodkz
Copy link
Member

nodkz commented Aug 9, 2018

🎉 This PR is included in version 4.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@toverux
Copy link
Contributor Author

toverux commented Aug 9, 2018

Oh noes, I forgot to add the build-ts task to the main build task :(
Worked in my tests because I used build-ts only before packing...
There's always something like that with distribution-related things.
I'm really sorry. Would you like me to open a PR for this, or do you prefer just editing the file yourself?

diff --git a/package.json b/package.json
index 569f262..106b172 100644
--- a/package.json
+++ b/package.json
@@ -84,7 +84,7 @@
     ]
   },
   "scripts": {
-    "build": "npm run build-lib && npm run build-mjs && npm run build-es && npm run build-node8",
+    "build": "npm run build-lib && npm run build-mjs && npm run build-es && npm run build-node8 && npm run build-ts",

@nodkz
Copy link
Member

nodkz commented Aug 9, 2018

Please open PR. From mobile now.

@nodkz
Copy link
Member

nodkz commented Aug 9, 2018

🎉 This issue has been resolved in version 4.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@toverux
Copy link
Contributor Author

toverux commented Aug 9, 2018

We're good with #116 !
Thanks a lot, @nodkz.

@nodkz
Copy link
Member

nodkz commented Aug 10, 2018

And thanks you too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants