waypoints
waypoints copied to clipboard
CommonJS modules
Here's my attempt to rewrite code in CommonJS modules. I've been using your waypoints a long time ago, and I'm happy to contribute.
This PR should solve issue #458. I didn't put files in lib/* because I don't like store code twice in repository. They can be easily added later.
+1 Is this going to be pulled yet? @imakewebthings
+1, totally would save me time
On first reading this looks good @summerisgone. Thank you. I'm currently in the middle of caring for a newborn so my time here comes in sporadic, short chunks. I'll try my best to use those chunks to test and merge this over the next few days.
@imakewebthings my congrats for your baby! Stay happy and healthy! I'm ready to discuss any solution, I've tried to keep compatibility with previous builds as much as possible.
@summerisgone The files you added to lib
, the test builds, did you edit them by hand after generating via npm run build
? I get some mostly minor differences when I build locally and want to make sure I'm not missing something.
@imakewebthings as I said, I excluded lib/* files from commit, to show only code difference for convenience. Added them now.
@summerisgone Here's the plan. I've merged this PR into a branch, 5.0. I very much like this PR, and it makes it painfully obvious to me that a couple things should change in concert with this CommonJS changeset. The key ones:
- No more separate shortcut files. Include it all into the main file, as you've done here. The downside of larger file size for things that not everyone uses is outweighed by the ease of use that comes with one simple file/require. This also leaves the door open for a custom build process in the future for the extremely bit-conscious users.
- No more jQuery/Zepto deps. That means in the shorcuts, and those two adapters can die. The extensions can still live on, but I believe it would be best to package it as a method one can call to decorate
jQuery.fn
. Like so:
Waypoint.extendJquery(jQuery)
jQuery('.blah').waypoint(...)
I intend to do that work on top of your changes in the 5.0 branch. Simultaneously, there are some bug fixes and such that I would like to release as 4.x following master. Those changes will be folded into 5.0 through rebasing as I go along.
@imakewebthings thank you for submission, I'm very glad again to contribute :) I have some questions about your plan:
- I didn't get, do you want to make all-in-one build or leave separate files? As third opinion, you can build two versions like full and minimal. I leaved shortcuts builds as is, and they depends on
global.Waypoint
so they can be just added on the page. - How do you plan to get rid from jQuery/Zepto deps, if some [useful] shortcuts depends on them?
I can try to rewrite code to
var jquery = require('jquery')
rather than usingwindow.jQuery
. But then the module (in CommonJS context) would depend on jquery, which lays to some overweight (so I leaved them as optional dependencies and used global context for variables).
@summerisgone The package.json main would point to an all-in-one build. If somebody were worried about file size and wanted to only include a subset of everything, there's nothing stoppng them from using require('waypoints/src/whatever.js')
and building that into their final bundle. The all-in-one mostly makes things 100x easier to explain. I'm also not a fan of leaking Waypoint
into the global namespace if a user is going the CommonJS route. Having the separate files would necessitate that or some other means of extending the Waypoint namespace.
For the jQuery/Zepto shortcuts, I intend to update those shortcuts to work without jQuery/Zepto.
Awesome plan @imakewebthings I hope it will become reality soon.
I agree so much. Waypoints is awesome, but using it with Webpack etc. can be a pain in the first point. CommonJS would solve it!
If you need assistance from us @ webpack ensuring this can be bundled properly let me know. We are willing to help.
Thank you for the offer @TheLarkInn, but the following official workaround is working pretty well for me. :)
However, I would strongly vote for switching to a wider supported module in the next future. Maybe even consider es-modules via babel to supply all common module formats out there.
Here is the workaround in case somebody else is looking for a fix:
plugins: [
new webpack.ProvidePlugin({
$: 'jquery',
jQuery: 'jquery',
'window.jQuery': 'jquery'
})
]
@axe312ger a PR on the readme would be really beneficial.
@summerisgone Here's the plan. I've merged this PR into a branch, 5.0. I very much like this PR, and it makes it painfully obvious to me that a couple things should change in concert with this CommonJS changeset. The key ones:
- No more separate shortcut files. Include it all into the main file, as you've done here. The downside of larger file size for things that not everyone uses is outweighed by the ease of use that comes with one simple file/require. This also leaves the door open for a custom build process in the future for the extremely bit-conscious users.
- No more jQuery/Zepto deps. That means in the shorcuts, and those two adapters can die. The extensions can still live on, but I believe it would be best to package it as a method one can call to decorate
jQuery.fn
. Like so:Waypoint.extendJquery(jQuery) jQuery('.blah').waypoint(...)
I intend to do that work on top of your changes in the 5.0 branch. Simultaneously, there are some bug fixes and such that I would like to release as 4.x following master. Those changes will be folded into 5.0 through rebasing as I go along.
Does that mean, that by default Waypoint won't use jQuery/Zepto or is it just the shortcuts?