gamecontroller.js
gamecontroller.js copied to clipboard
Migrate to Typescript
- Migrate to Typescript
- Add builds for
- types - Optional for those who use Typescript
- browser - The Webpack bundle
- ESM - Optional for those who want to use gamecontroller.js as a module in there own bundler
- CommonJS - Useful if you want to use gamecontroller.js in node e.g. with JSDOM, Jest or Webpack (Is now also used internally for the Jest tests)
- Add Github action to run the Jest tests on any commit, see example
- Update tests and examples so that everything continues to work
- Add
servescript to run the examples locally - Add a webpack.config.js to transpile Typescript using Babel for the browser bundle output
Hi @JumpLink, thank you for creating this PR. Migrating to Typescript was not in the roadmap in the short/mid-term. I will need to review these changes thoroughly (it is a big change) and that will take time.
@alvaromontoro yes I understand. I thought it would make more sense (and not really more effort) to port the project itself to Typescript instead of writing separate types for @types.
Thanks to the typescript precompiler, it was also easy to build the package for ESM and CJS, so I did that as well.
To stay backwards compatible I left the webpack build in, so this bundle can still be used if you don't want to use npm or any bundler in your own project.
So the result is:
- Project migrated to Typescript
- Packages supports ESM, CJS and a minified bundle direct for the browser
- And the other things I mentioned above that came up e.g. testing the tests etc
Possibly the types can be further improved and thanks to Babble the APIs for old browsers can also be omitted in the future. For this I also had to use the any type in TypeScript, since the old APIs are no longer available in the DOM TS types. I left them in so as not to damage anything
I would be happy if the PR would be merged and I already have another idea for a future separate PR.
Sorry, it's taking me so long. At the moment, I don't have the bandwidth to take care of such a big review :(
@alvaromontoro Yes I understand that, don't stress yourself. Since the codebase is manageable I would just look at and test the result instead of review the PR's diff directly.
At the moment I'm busy with other projects myself but in the future I have some more features in mind that I would like to submit. For example, Firefox seems to map some controllers differently than Chrome on Linux. I haven't tested if this is also the case on other operating systems, but we could add the possibility to make the mapping of the buttons configurable ( maybe depending on the browser /os) and in this way align the controller mapping of Firefox.
Do you have a kind of roudmap so that I can assess which features you have already planned for yourself in the future?
However, I would then want to submit such a PR based on this PR. Therefore, I would be happy if the review is completed by then.
@alvaromontoro What is the status on this? It's been 1 year already. I don't think you need to "thoroughly review all the code" as it is just a TypeScript port (to generate typings), @JumpLink probably took the time and effort to rewrite your code and adding types actually reduces the chance of having major breaks. If it's ok, you just have to run your existing tests and if everything is ok it would be nice to bump the update on npm!
@alvaromontoro If you are not sure about how this update will play out, you could accept the changes on a separator branch (e.g. pre-typescript) so your main branch remains unchanged.
And then push this typescript version onto npm using the same tag so we can install it using a syntax like
npm i -D gamecontroller.js@pre-typescript
Note: Another big advantage of TypeScript is that you can deliver the doc directly into the declaration files (.d.ts),
So for instance if you start typing gamepad.on(' your IDE will show you the available events and assuming we add the documentation into the declaration file we could know for instance that button0 is A on the Xbox controller and so on (without the need to always go back and forth between code and controller map image.)
@JumpLink By the way, it might help the review if you remove the dist directory from this PR.
It's not recommended to stage production files in the repo, if you include the build steps in the package.json then the author can generate the dist files on his end and publish them with npm publish.
Please consider doing that as soon as you can, so maybe it may help the author to look and accept this PR or not.
@JumpLink Ok I couldn't resist to make my own esm version https://www.npmjs.com/package/esm-gamecontroller.js
If you are interested in making some changes, let's continue the work there!
Have a good one.
@vdegenne thank you for this!