Skip to content
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

Updating this code to Ink 2 #3

Open
john-osullivan opened this issue Dec 9, 2019 · 3 comments
Open

Updating this code to Ink 2 #3

john-osullivan opened this issue Dec 9, 2019 · 3 comments

Comments

@john-osullivan
Copy link

Hello, I'm in the middle of writing a CLI which needed something just like this. However, I'm working in Ink 2, so this component doesn't fly. After reading through the source code, I decided that the underlying primitives are different enough to merit a full rewrite.

I've just about completed that rewrite here https://github.com/Eximchain/ink-quicksearch-input/ :

  • Uses React's new function components and Ink's new hooks
  • Written in Typescript, so props get typechecking in VS Code
  • Internal function names are now oriented around the allowed user interactions (e.g. addCharToQuery, selectUp)

Were you planning on doing this upgrade yourself at some point? Would you want to try and merge this new code into your existing base, or just keep them separate?

@john-osullivan
Copy link
Author

Hallo, happy new year! Any chance you've been able to look this over? The main ink maintainer would prefer that this original npm component got updated to the new version, rather than adding a whole new library. I'll see about making a PR with the changeset.

@aicioara
Copy link
Owner

aicioara commented Jan 21, 2020

@john-osullivan, sorry, I totally missed your issue. If the current version of ink-quicksearch is no longer compatible with ink v2, I am definitely interested in fixing that. I am less interested in hooks, unless they offer a significant advantage (performance or otherwise). I am less interested in a TS rewrite, but really excited to add .d.ts files if that suffices. (But that's just my opinion, happy to discuss tradeoffs).

@john-osullivan
Copy link
Author

(Replying to this on the PR for the sake of keeping everything in one place)

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

No branches or pull requests

2 participants