naver / hammer.js

A javascript library for multi-touch gestures :// You can touch this
http://hammerjs.github.io
MIT License
43 stars 11 forks source link

No event is fired when using together with webpack #6

Closed tedbarth closed 6 years ago

tedbarth commented 6 years ago

While I can compile my webapp now using webpack the lib now refuses to fire any event. I created a new issue since it is a totally different bug.

I made a very simple example app to show the problem to you: tedbarth/hammerjs-demo

It uses both your fork (2.0.12) and the original hammer.js (2.0.8). With the original library the box can be dragged (pan event), but not with your fork. I tested multiple browsers and could find no one that could ~hammer~ handle it. You can compare the implementations when toggling the first two lines of the index.js file (Don't forget to rerun webpack!):

import Hammer from '@egjs/hammerjs';
// import Hammer from 'hammerjs';
[...]

Thanks in advance!

jongmoon commented 6 years ago

@tedbarth Thank you again 👍 Your example would helps to fix this problem. I'll check it.

jongmoon commented 6 years ago

Reason for fail

We remove the dependency that constant(like DIRECTION_ALL) exist in Hammer. because We want Hammer.js to be tree-shaked.

But it makes side effect like you reported.

Hammer is just interface of core Manager, and If you does not use Hammer and just use Manager, then Tree Shaking reduces the destfile size from 22kb to 17kb when using '@egjs/hammerjs`.

Temporary Solution

If you need urgent fix. Following code will fix the promblem.

import Hammer, {DIRECTION_ALL} from '@egjs/hammerjs';

hammer.get('pan').set({direction: DIRECTION_ALL});// instead of `Hammer.DIRECTION_ALL`

Fundamental Solution

I think it is right to preserve the original hammerjs interface. So we will do a job that making constant to be in Hammer only when Hammer is referenced.

We will notify you when it is fixed. So following will be available after fix

Hammer.DIRECTION_ALL // 2.0.12 - undefined
// After fixed.
Hammer.DIRECTION_ALL // 2.0.13 - valid value

Thank you! If you did not report, we would not have found this problem.

jongmoon commented 6 years ago

@tedbarth 2.0.13 is release on npmjs. Could you download it again?

https://www.npmjs.com/package/@egjs/hammerjs

tedbarth commented 6 years ago

@jongmoon Thx a lot for the fix. It's working! :+1: BTW: You are doing a great job with explaining why things fail and how to solve it. This helps a lot to learn new things and encourages one to take part in the project. Thank you!

With Firefox Mobile I have noticed a different problem now (Vertical scrolling is not disabled on pan down/up), which I can't isolate yet. Simple tests seem to work though. Don't know what triggers the bug, yet. Will create a new issue when I'm able to reproduce it in an isolated demo.

jongmoon commented 6 years ago

@tedbarth If you help~ I believe we can improve this library together! 👍