search

JakeSidSmith / react-fastclick

Fast Touch Events for React
https://www.npmjs.com/package/react-fastclick
MIT License
487 stars 41 forks source link

Added conditional initialization #42

Closed dutzi closed 7 years ago

dutzi commented 7 years ago

See #41

JakeSidSmith commented 7 years ago

I see what you're going for, would like to keep the IIFE wrapper though, and add some better export support, like in https://github.com/JakeSidSmith/react-reorder/pull/68/files. This way people can add as a script tag, import with AMD, etc.

JakeSidSmith commented 7 years ago

Would you like to make these changes, or shall I open another PR?

dutzi commented 7 years ago

Sure, I'll give it a try.

dutzi commented 7 years ago

I made the change you asked for (I hope it's ok, I'm not too familiar with AMD/CommonJS import mechanisms).

But I ran into the following eslint error which I couldn't solve:

/Users/dutzi/Documents/wix/react-fastclick-fork/lib/index.js
  247:33  error  'React' is already declared in the upper scope  no-shadow
JakeSidSmith commented 7 years ago

@dutzi For the AMD imports I just give them a different name within that scope e.g.

define(['react'], function (ReactAMD) {
dutzi commented 7 years ago

Ok, so I kept it as is (no bind) and just renamed React as ReactAMD.

JakeSidSmith commented 7 years ago

Looks good. Gonna merge this in a bit, and make some other small changes, then I'll get a release out. :)

dutzi commented 7 years ago

Cool :)

JakeSidSmith commented 7 years ago

@dutzi published v3. :)

dutzi commented 7 years ago

Yey!