search

jayphelps / core-decorators

Library of stage-0 JavaScript decorators (aka ES2016/ES7 decorators but not accurate) inspired by languages that come with built-ins like @​override, @​deprecate, @​autobind, @​mixin and more. Popular with React/Angular, but is framework agnostic.
MIT License
4.52k stars 263 forks source link

TypeScript beginnings #123

Closed BurtHarris closed 7 years ago

BurtHarris commented 7 years ago

Add support for building project with TypeScript

This saves the gems from my experimental branch, without adopting some of the mistakes. It enables you to either type tsc or npm run build-tsc to build all the source files from this project into a new built directory. This is intended to facilitate comparison with the babel generated code, and add some error checking.

Note that the error checking is currently fairly minimal, as the its using --allowJs, but not --checkJs. I've also included in this PR code fixes for some of the error reported by tsc the command generally results in a clean build.

BurtHarris commented 7 years ago

Fixes #124

BurtHarris commented 7 years ago

I'm investigating this CI failure...

BurtHarris commented 7 years ago

It appears there are some cases where using the namespace-style import doesn't work, even in babel. The test.spec.json file, as small as it is, is now preventing the typescript-based test from running. I gather it's related to sinon-chai. All the babel tests should be fine again.

@jayphelps, I'm going to let this sit now, please give it a code review if/when you get a chance.

BurtHarris commented 7 years ago

P.S. If after building both the babel and TypeScript outputs, if I move the TypeScript-produced built\lib into the root directory's lib, and run Mocha, the tests all 73 pass. I offer the hypothesis that the majority of the TypeScript related problems are now in the test code, rather than in the library.

BurtHarris commented 7 years ago

OK @jayphelps, I think this is ready for another code review.

Change in build scripts makes build-tsc build directly into the lib/ directory. So:

npm run build-tsc
npm test

runs the tests. I'm currently getting 20 errors from the build-tsc variant. Many seem related to decorators you are planning on removing.

BurtHarris commented 7 years ago

@jayphelps I've made suggested changes, thanks. Do I now click the "Dismiss Review" link?

jayphelps commented 7 years ago

I've made suggested changes, thanks. Do I now click the "Dismiss Review" link?

Nope, you can either request another review (if it lets you) or ping the reviewer to review the changes. They will then either approve or submit additional requested changes. 😄

jayphelps commented 7 years ago

Thank you!! 🎉 🎈 :man_dancing:

I need to find some time to really dig deeper and confirm this won't break anyone if I release a build with this. I don't imagine it will, but tbh I haven't looked at the actual builds look like yet and just cause the tests pass doesn't mean it'll work :trollface: If I don't comment here about a release in the next week, feel free to bug me. Gonna try to find the time this weekend.