search

filamentgroup / Overthrow

A tiny, no-frills, framework-independent, targeted overflow: auto polyfill for use in responsive design.
MIT License
906 stars 94 forks source link

0.7.1 should be 0.8, if not 1.x #91

Closed shlomiassaf closed 1 year ago

shlomiassaf commented 8 years ago

Changing the module name causes broken references in builds using commnJS.

This is an API breaking change.

scottjehl commented 8 years ago

hmm. Okay, I'd agree but this plugin does not yet export to commonjs (that's on the horizon but I haven't done it yet). It wasn't published to npm yet either. This change made for the first npm push, since there's another package published under the overthrow name on npm.

Make sense or am I missing the point? Thanks

shlomiassaf commented 8 years ago

You're right but with smart bundlers, such as Webpack, its easy to overcome packages without commonJS support.

This was my previous "require": require('imports?this=>window!overthrow/src/overthrow-detect.js');

After you change today, all my CI builds failed, had to change to: require('imports?this=>window!fg-overthrow/src/overthrow-detect.js');

Every webpack project, using your package as npm module via github link will fail now since version change was a PATCH...

BTW, once you support commonJS, its also a breaking api change, I will have to remove the loader that sets "this" to window.

scottjehl commented 8 years ago

That’s good feedback, thanks. And helpful to see your web pack setup.

I’ll change the version to 0.8 and deprecate this one. Sound good?

On Nov 17, 2015, at 9:24 AM, Shlomi Assaf notifications@github.com wrote:

You're right but with smart bundlers, such as Webpack, its easy to overcome packages without commonJS support.

This was my previous "require": 'require('imports?this=>window!overthrow/src/overthrow-detect.js');'

After you change today, all my CI builds failed, had to change to: 'require('imports?this=>window!fg-overthrow/src/overthrow-detect.js');'

Every webpack project, using your package as npm module via github link will fail now.

BTW, once you support commonJS, its also a breaking api change, I will have to remove the loader that sets "this" to window.

— Reply to this email directly or view it on GitHub https://github.com/filamentgroup/Overthrow/issues/91#issuecomment-157402332.

shlomiassaf commented 8 years ago

Yep, little bit more work but its the right way