scniro / react-codemirror2

Codemirror integrated components for React
MIT License
1.65k stars 192 forks source link

typescript definition #20

Closed agdolla closed 6 years ago

agdolla commented 6 years ago

Hi Do you plan to create a typescript definition ? I'm using @types/react-codemirror but I needed to change it as your module exports different object

scniro commented 6 years ago

Hello, I am working on this now and plan to be done soon (day or so). I'll update here once it's available.

Actually, before I publish up a new version would you be willing to help test out the changes privately to make sure all looks good (it's possible I'll miss something here or there)?

agdolla commented 6 years ago

@scniro if you send it I can test it, too

scniro commented 6 years ago

@agdolla Hey there. So I took a stab at this and spun up a new repo at react-codemirror2-types. This contains the packed up .tgz package. You can download it and install manually as if it were available on npm, but you'll instead include the file in your source and npm install react-codemirror2-3.0.0-alpha.tgz

I'm not going to publish up to the DefinitelyTyped repo, but instead ship this component with the typings included. I'll plan to bump a new major version if this looks good to us after some use. A second set of eyes is super useful so I appreciate your willingness to test with me.

I stumbled across a few interfaces with missing/incorrect signatures in @types/codemirror so I'll plan to get a PR sent their way to fix what I discovered. Regardless, those won't hold up your testing but only my source development.

Look forward to hearing back about your findings!

agdolla commented 6 years ago

@scniro Hi I tried to used, but how do you specify a mode? I mean in js you have this:

<CodeMirror
  value='<h1>I ♥ react-codemirror2</h1>'
  options={{
    mode: 'xml',
    theme: 'material',
    lineNumbers: true
  }}
  onChange={(editor, metadata, value) => {
  }}
/>

in typescript there is no options defined

also

export interface IDefineModeOptions {
    name: string;
    fn: () => any;
}

fn could be something like

fn: () => codemirror.Mode
scniro commented 6 years ago

I totally overlooked options. I was trying to get all the callbacks and most props typed. I'll work through options today. I also like the fn: () => codemirror.Mode suggestion. I'll try to get a new version pub'd up to the test repo later today

scniro commented 6 years ago

@agdolla I pushed up a new binary to the test repo, could you install and let me know if this revision looks better? In the meantime, I have begun to write some unit tests to ensure I didn't break anything. I am actually having a bit of difficulty doing this because of the nature of the library rendering the editor on a ref (something about portals). I'll keep going, but I'll likely just spend some time manually testing everything.

If this looks good, I'll publish up to the main repository so the types will come down with the new version. If you have any more suggestions please keep them coming 😎

agdolla commented 6 years ago

LGTM :+1:

scniro commented 6 years ago

@agdolla hey I wanted to let you know that I haven't forgotten about this. I came across a few annoyances while getting the types all worked out and I wanted to take the opportunity to tackled everything at once. You can check out #22 for a little about what I have going on. I am hoping to roll out the new changed in a 3.x release in the coming days.

scniro commented 6 years ago

@agdolla This should now be fixed in 3.0.0.

I changed the core API a bit, please check out whats new in 3.0.0 to get up and running.

Please let me know if this is resolved! 😸

agdolla commented 6 years ago

@scniro sorry I did not have time to test it :(

scniro commented 6 years ago

@agdolla It's all good, we all have a crammed schedule. There haven't been any drastic changes to the typings since you last tried it out. I opted to close this for now but please re-open should you encounter any difficulties. Thank you for the collaboration!

PatrickKunz commented 6 years ago

Deleted comment.

Forget my comment. I forgot to import the code mirror types. Works all as expected. Thanks for your hard work :)