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

Add typescript types to types repo #275

Closed
johnymontana opened this issue Jul 8, 2019 · 27 comments · Fixed by #597
Closed

Add typescript types to types repo #275

johnymontana opened this issue Jul 8, 2019 · 27 comments · Fixed by #597

Comments

@johnymontana
Copy link
Member

Add types for all neo4j-graphql.js exports to https://github.com/DefinitelyTyped/DefinitelyTyped

@Pruxis
Copy link
Contributor

Pruxis commented Jul 9, 2019

What would you think of using typescript in this repo instead?

I find it very difficult to find my way around in the codebase to solve issues in the recursion, a fully typed codebase would help with that.

@ilmimris
Copy link

What would you think of using typescript in this repo instead?

I find it very difficult to find my way around in the codebase to solve issues in the recursion, a fully typed codebase would help with that.

I'm looking for a fully typed codebase, where do I can found it?

@AdrienLemaire
Copy link
Contributor

@johnymontana @michaeldgraham is there an estimated deadline to release a declaration file for neo4j-graphql-js?
I can find a changelog for the project, but not a roadmap (the project is also a bit empty). It's a bit unclear what are GRANDstack goals going forward.

Until then, has someone written a .d.ts file that I could plug to my project? That would be very helpful if you could share it, thanks in advance :)

@Nopzen
Copy link
Contributor

Nopzen commented May 30, 2020

👍 here as well. I started making a neo4j.d.ts in my own project but unsure how well I will end up typing it, but if it becomes something I will create a pr, in the mean time if some one is faster than me feel free.

edit
Since this will be my first contrib to @types/ as well it might take some time.

@Nopzen
Copy link
Contributor

Nopzen commented May 31, 2020

For those interested, I've created a "transparency" PR in my fork of DefinitelyTyped, where i would like some input from the contributors on my types, as im not a super user of this project but would like to contribute:

https://github.com/Nopzen/DefinitelyTyped/pull/1

feel free to comment and help out.

@AdrienLemaire
Copy link
Contributor

AdrienLemaire commented Jun 3, 2020

Hey @Nopzen, sorry about the newbie question. How should I go about importing your declaration file in my project?

I copied your code to a local neo4j.d.ts file, and in the file importing the library, I added a triple-slash reference to the .d.ts

/// <reference path="./neo4j.d.ts" />
import { neo4jgraphql } from "neo4j-graphql-js";

But that doesn't work. And the error asks me to do exactly what I did 🤔

This is probably related to the error shown on the declare module "neo4j-graphql-js" { line in the neo4j.d.ts file:

 [tsserver 2665] [E] Invalid module name in augmentation. Module 'neo4j-graphql-
 js' resolves to an untyped module at '/home/dori/Projects/Work/GOunite-v2/v3/
 api_server/node_modules/neo4j-graphql-js/dist/index.js', which cannot be
 augmented.                                                                     

Edit: Fixed after reading this SO thread by moving the import within the declare scope

@Nopzen
Copy link
Contributor

Nopzen commented Jun 3, 2020

@AdrienLemaire thanks for helping me with those findings, also this have now been fixed in my latest commit in the PR. instead of just copy pastaing the code i think NPM can install a module directly from github:

https://medium.com/@jonchurch/use-github-branch-as-dependency-in-package-json-5eb609c81f1a then you can just do npm install to get the latest version i do when ever i publish something new, I will try and get around to define some more modules tonight after work.

@Nopzen
Copy link
Contributor

Nopzen commented Jun 3, 2020

@johnymontana & @Pruxis would you guys be able to take a look at my PR to see if these kinda types will suit this project, this is my first time I'm typing another open source project:

https://github.com/Nopzen/DefinitelyTyped/pull/1

I've tried to type out whatever that was documented in GRAND Stack API Refs and Graphql.

Feel free to come with feed back,

@Nopzen
Copy link
Contributor

Nopzen commented Jun 7, 2020

@AdrienLemaire did you get a chance to use the types?

@AdrienLemaire
Copy link
Contributor

AdrienLemaire commented Jun 8, 2020

not really ^^ I unfortunately can't spend much time on my grandstack project

Also, I couldn't figure out how to install the types/neo4j-graphql-js part of your Nopzen/DefinitelyTyped#feature/neo4j-graphql-js fork of @types with npm install.
Maybe the following, but it complains:

$ npm install --save-dev Nopzen/DefinitelyTyped/types/neo4j-graphql-js#feature/neo4j-graphql-js
npm ERR! code ENOLOCAL
npm ERR! Could not install from "Nopzen/DefinitelyTyped/types/neo4j-graphql-js#feature/neo4j-graphql-js" as it does not contain a package.json file.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/dori/.npm/_logs/2020-06-08T04_52_50_019Z-debug.log

@Nopzen
Copy link
Contributor

Nopzen commented Jun 22, 2020

@johnymontana - I would really love to hear back from you, if you like these types, and where you would like them to live? some neo4j products the types lives with the project and some it lives in DefinitlyTyped.

I can make PR's against any of the projects, but before doing this I would like some feedback from the owners / authors of the project.

@Nopzen
Copy link
Contributor

Nopzen commented Dec 29, 2020

My PR is still open, and a few people have added comments to this, I'm still looking for people to test these types I've added, as im no longer using Neo4j for my graphql project for now :) but if any is still interested in fixing this, I'm still able to help out, but might be a bit delayed in my answers now and then.

@lirbank
Copy link

lirbank commented Feb 4, 2021

Hey, it would be ideal if this package would include TS types instead of adding them to DefinitelyTyped.

https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#including-declarations-in-your-npm-package

@lirbank
Copy link

lirbank commented Feb 4, 2021

I use the typings @Nopzen wrote files with some modifications, seems to work well. It would be awesome if Neo4j maintained the typings so we don't have to maintain them in our own codebase.

https://github.com/Nopzen/DefinitelyTyped/pull/1/

@Nopzen
Copy link
Contributor

Nopzen commented Feb 5, 2021

@lirbank Happy to hear that some one is using them, please feel free to make a comment on the PR i got going if you wanted your changes in the types so you don't need to make your own local changes, if they are generic enough everybody should benefit from this.

@Nopzen
Copy link
Contributor

Nopzen commented Feb 15, 2021

@johnymontana Would you be open to a Pr directly to the neo4j-graphql-js repo? (I would love to contribute here) or do you just wanna make it live in the massive @types monorepo ?

I think honestly it would be nice to have it here, and I would not mind helping out maintaining this, with feedback from users, but it would be lovely if we could get some feedback from the Neo4j team as well on this?

@gpaluk
Copy link

gpaluk commented Feb 20, 2021

@johnymontana We would really appreciate this PR being merged and maintained. We are using TypeScript and types provide a real positive productivity impact. Thank you @Nopzen for your work and continued PR requests.

@Nopzen
Copy link
Contributor

Nopzen commented Mar 3, 2021

Minor update, as stated in my fork of DefinitlyTyped, I'm wrapping this up asap as more and more people are starting to use my fork, and i find it unfair that they are not using the main mono repo. I will read up whats the requirements is to merge this now to DefinitlyTyped, I hoped i could have made a contribution here, but the lack of involvement over 1 year makes this stale, and i no longer use this lib my self, I wanna give the community what they already provided positive feedback on.

Once merged to DefinitlyTyped, Hopefully this issue could get closed.

@nikosantis
Copy link

Minor update, as stated in my fork of DefinitlyTyped, I'm wrapping this up asap as more and more people are starting to use my fork, and i find it unfair that they are not using the main mono repo. I will read up whats the requirements is to merge this now to DefinitlyTyped, I hoped i could have made a contribution here, but the lack of involvement over 1 year makes this stale, and i no longer use this lib my self, I wanna give the community what they already provided positive feedback on.

Once merged to DefinitlyTyped, Hopefully this issue could get closed.

Nice!!! need use this!.

Is time to neo4j-graphql-ts?
I have also seen little movement and interest from the neo4j team. For example, with the update of thegraphql dependency.

@johnymontana
Copy link
Member Author

Thanks so much @Nopzen for your work on this! And apologies for the radio silence from my end.

My suggestion of adding types to the DefinitelyTyped monorepo was based on the perception that is the preferred standard - perhaps I was wrong about that? If you run into barriers merging the PR in to DefinitelyTyped we could include the types in this repo, I'm not sure what would be preferable for users?

More generally - work has begun on an official Neo4j GraphQL integration that is written in TypeScript and includes many new features. It is currently available as an early alpha release in @neo4j/graphql If you're in the Neo4j Slack workspace the #graphql-alpha channel has recently formed to discuss this new library in more detail.

@Nopzen
Copy link
Contributor

Nopzen commented Mar 4, 2021

@johnymontana thank you for your feedback, I started the process Yesterday to create a PR to the DT Monorepo bu it's a bit funky as my types are depending on packages this lib. Is depending on, it is not the defacto standard actually, repos like graphql and Apollo graphql includes their own index.d.ts

Also a small chat with a DT maintainer reveled only 0.5% of all npm packages are covered by DT :)

I would much rather also commit this single index.d.ts file here, as I would love to contribute to this package.

I would love to create a PR her for you if you have the time to go over it, If you haven't seen the larger discussions in my Pr with other community members feel free to visit my current pr.

I think for the ease of finishing being open for over 1 year, we could just put the type definitions here? And all users would just need a minor package bump to use it?

@johnymontana
Copy link
Member Author

@Nopzen Thanks for clarifying. Yes, I agree adding the index.d.ts here makes sense in this case. As others are already using your DefinitelyTyped fork I think that is good validation and we can go ahead and add it here.

Could you open the PR?

@Nopzen
Copy link
Contributor

Nopzen commented Mar 8, 2021

@johnymontana Yes, I will do so today when i get 5 mins :)

@Nopzen
Copy link
Contributor

Nopzen commented Apr 21, 2021

@johnymontana would it be possible to maybe have one your guys to have a look over the pr created possible @danstarns maybe ❤️

@danstarns
Copy link
Contributor

@Nopzen Thanks for all the work your putting into this! I don't have much influence over this library but I do see the value in providing types for the library although do consider the new implantation, that is typescript first, https://github.com/neo4j/graphql.

@Nopzen
Copy link
Contributor

Nopzen commented Apr 22, 2021

@danstarns I just remembered you had done some work on the typescript first lib, that's why I tagged you here :)

Like I stated in the PR I will try and keep my fork up to date with master until this get merged, so people who used this can use my fork.

Hopefully it would get merged soon as it's a old issue :D

@michaeldgraham
Copy link
Collaborator

#608

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 a pull request may close this issue.

10 participants