search

dorukeker / gyronorm.js

JavaScript project for accessing and normalizing the accelerometer and gyroscope data on mobile devices
MIT License
641 stars 65 forks source link

Making gyronorm ES6 / webpack friendly #29

Closed superhighfives closed 7 years ago

superhighfives commented 7 years ago

First off, thanks for making GyroNorm.js—it's awesome, and I've really enjoyed using it with Sandpit.

I was having a look through the issues and I noticed https://github.com/dorukeker/gyronorm.js/issues/27, so I dug into it a little. The issue for ES6 is that GyroNorm doesn't package FullTilt with the main: JS file denoted in package.json. This means that FullTilt needs to be installed as a separate dependency on any project that uses it, and imported before GyroNorm is.

There are a few ways to approach resolving this, but the quickest solution without rebuilding GyroNorm would be to change the main entry point in package.json to point to dist/gyronorm.complete.js.

So, that's what this pull request does. The benefit is people can still import a non-FullTilt version in ES5 or ES6, but this will ensure the library works out of the box for anyone who is using require or import.

I should add, when I installed dependencies and built it appears to have retrieved newer versions of the dependencies, so I had to update some of the references in the Gruntfile. You could stick with your current build and just update the package, or merge and use this.

And again, I really appreciate all your hard work on GyroNorm. 👍

superhighfives commented 7 years ago

Okay, so it turns out the es6-promises shim is doing some funky stuff around dependency checking—which will break builds and test environments.

Having thought about it some more, I believe including fulltilt is more important than including promise support, because promises can be covered by a range of polyfills. Many projects polyfill already, or can do so on a case by case basis.

So, I propose the library offers the following builds:

The package file could then point to dist/gyronorm.js and work out of the box, while dist/gyronorm.complete.js would be available to those needing it.

If someone still needed gyronorm only, they could grab it from lib/gyronorm.js.

Thanks for taking the time to review, and cheers again.

dorukeker commented 7 years ago

Hello, I'm glad you find it useful and thanks for the kind words. Indeed I like your initial solution more: to change the entry point to the .complete. version.

Also semantically it makes more sense. If it is complete it contains everything. What about we use a different promise polyfill? e.g. https://github.com/dmarcos/promise

I dont have a quick setup I can test. Can you test if it works? This way we can solve the issue and keep the naming intact.

Cheers, Doruk

superhighfives commented 7 years ago

Awesome! Thanks for merging, and again, for all your hard work. The new promise polyfill is much simpler, too.

👍

dorukeker commented 7 years ago

I thank you for taking the time for the PR.

Couple of further changes I plan to make.

Cheers, Doruk

superhighfives commented 7 years ago

I wasn't able find an ES6 friendly version of FullTilt on NPM, which is probably why I was leaning towards to being in the core. Which reminds me: I should open a PR to the current FullTilt on there to add a proper main entry point.

Which browser was the promise polyfill failing for you in? Did it give a specific error message? I may have inadventently tested it in an enviornment that natively supported Promises, and missed it. Apologies if that's the case.

I agree that removing the polyfill is reasonable—it has pretty good browser support, and projects that need it can polyfill anyway. In that case, I'd make complete FullTilt and Gyronorm, and core just Gyronorm, which would solve the first point too! Okay, ignore everything prior to this paragraph! Haha. ☺

dorukeker commented 7 years ago

:) I will have updates soon. Thanks again. AFAIK FullTilt is not managed in open source anymore. (https://github.com/adtile/Full-Tilt/issues/19) I am still using an older version of it. So non of us to have any license issues ;)

superhighfives commented 7 years ago

Yeah, I just noticed that! What a shame. Great you can still use an older version though. 👍

superhighfives commented 7 years ago

I'd be happy to make the changes if you need. Otherwise, looking forward to it! Thanks again.

simonbuehler commented 7 years ago

hi guys, trying to get this working using webpack but the changes here (fulltillt included) somehow dont show up in the latest npm ("version": "2.0.6")

how can i fix this?

edit: i think the problem is that not the gyronorm.complete.js is included but just the default one, wher do i have to change it so it uses the complete file?

dorukeker commented 7 years ago

Indeed NPM acts strange. "$ npm info gyronorm" returns version number as 2.0.6, which is correct. But when I install the package 2.0.4 is installed. Like wise in their web site the version numbers are different in the general list and in the detail page. I will check with them.

In the meantime you can fork/download from Github. Or use something like this "$ npm install https://github.com/dorukeker/gyronorm.js/v2.0.6"

simonbuehler commented 7 years ago

yep, changed "main": "dist/gyronorm.complete.js" in node_modules/gyronorm/package.json, but thats a hacky fix

dorukeker commented 7 years ago

Yes indeed. As soon as I hear from NPM people, I will update here.

mrisher commented 6 years ago

Hi: Did this get updated? I just tried gyronorm using webpack (Vue) and received the same issue around FULLTILT not being installed (because import 'gyronorm' doesn't pull in the .complete version)