IjzerenHein / kiwi.js

Fast TypeScript implementation of the Cassowary constraint solving algorithm 🖖
Other
249 stars 24 forks source link

Switch from Typescript namespaces to ES6 modules #7

Closed adamhaile closed 5 years ago

adamhaile commented 5 years ago

NOT READY FOR MERGE

This PR switches kiwi.js from the older Typescript namespace syntax to standard ES6 module syntax (import/export).

Advantages of ES6 modules include:

Rollup is added to combine kiwi.js's ES6 modules into a single UMD module.

Current status is that it builds and passes all tests. Minified and JSDoc assets haven't been updated yet.

To run the new build, run npm run rollup. Take a look and see if you're comfortable with the general approach. If you are, I'll replace the grunt build with the rollup one as well as add the steps to build kiwi.min.js and Kiwi.md.

codecov-io commented 5 years ago

Codecov Report

Merging #7 into master will increase coverage by 2.44%. The diff coverage is 86.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #7      +/-   ##
==========================================
+ Coverage   71.22%   73.66%   +2.44%     
==========================================
  Files           1        1              
  Lines         827      638     -189     
  Branches      124       94      -30     
==========================================
- Hits          589      470     -119     
+ Misses        238      168      -70
Impacted Files Coverage Δ
lib/kiwi.js 73.66% <86.36%> (+2.44%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8ca847f...967d7ea. Read the comment docs.

adamhaile commented 5 years ago

Docs and minified build now work.

Some notes:

Alternative: if you're uncomfortable/unfamiliar with rollup I could change the typescript config to output AMD modules, which can then follow your old strategy of concat and wrap with a UMD loader.

This ended up being a bigger set of changes than I initially anticipated. Very little code is changed in the core lib, but the build is different. If this is more than you're comfortable with, we can discuss intermediate strategies. Or you can just ditch the PR. It's your library :).

To disclose my interest here, I'm evaluating kiwi.js for use in my startup's product. I was attracted to it for speed, clean design and indicators of code quality (tests, docs, activity, etc). We definitely need Typescript typing support, which is why the existing package doesn't work for us (see issue #6). We prefer a modern build, as if we go with kiwi.js, I suspect we'll need to get to know its internals. We can run our own fork but if our changes are consistent with your direction for the library it makes sense for us to contribute upstream, as always with open source.

IjzerenHein commented 5 years ago

Hey man, this is great work, I will try to review and merge this shortly! Cheers, Hein

IjzerenHein commented 5 years ago

Hi, I've merged all your code, looks really good! I just removed the es folder from the git repo. It's still published in the NPM package, but I always want to prevent auto-generated code to be contained in the git repo. Thanks so much for this PR, this paves the way for the future. Cheers dude!