vmarchesin / react-konami-code

Trigger an easter egg by pressing a sequence of keys.
MIT License
62 stars 12 forks source link

Feat: add typescript type #2

Closed thecampagnards closed 5 years ago

thecampagnards commented 5 years ago

Hello,

This PR implement typescript compatibility. If you have any remarks I will fix it.

Konstantin

vmarchesin commented 5 years ago

Everything looks fine, and since it just implements typing the tests passed without any problems. If you want to you can go ahead and bump the minor version (and the CHANGELOG, maybe the README too) and I'll publish it so people can use this with TS. Let me know and I'll merge this PR.

thecampagnards commented 5 years ago

@vmarchesin ok i will do it !

vmarchesin commented 5 years ago

All right, just a quick question since I'm not familiar with TS modules on npm. Doesn't the package.json require the types property in order to enable TS compatibility? If I just put the .d.ts file to the dist folder on npm will it be enough?

By the way you changed the patch version (1.2.3 -> 1.2.4), not the minor version (1.2.3 -> 1.3.0) that should be changed because this is a new feature that adds backward compatibility, not a bugfix. Also the change isn't reflected on the package.json, which is actually the only thing required. I can fix this later, don't worry. Could you please just take a look on how to properly publish the package using TS and make the correct adjustments? You can use npm link to test the package locally on any project to see if it is TS compatible without the need of publishing it.

If there's no extra changes needed then just let me know if I just need to include the .d.ts file in order for the TS compatibility to work. If everything is working please provide a simple checklist for me to test this in a TS environment.

vmarchesin commented 5 years ago

As a reference take a look at this http://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

thecampagnards commented 5 years ago

Yep my bad, I forgot the package.json I fixed it. I tested it locally and everything is fine. You just have to put the d.ts file in the dist folder. To test it you can create a typescript react app https://www.npmjs.com/package/react-scripts-ts then change the App.tsx by the README.md example:

import * as React from 'react';
import Konami from 'react-konami-code'

export default class App extends React.Component {
  public render = () => (
    <Konami action={this.easterEgg}>
      Hey, I'm an Easter Egg! Look at me!
    </Konami>
  )

  private easterEgg = () => {
    alert("Hey, you typed the Konami Code!")
  }
}
vmarchesin commented 5 years ago

All set, I just tested and it seems to be working just fine. I'll publish the new version.

Thanks a lot for the addition :)