Skip to content

Conversation

@realityking
Copy link

This is a breaking change as the path to the dist file changed and closest.js is no longer externally accessible.

@realityking realityking mentioned this pull request Dec 9, 2017
2 tasks
@mrmartineau
Copy link

@realityking this looks cool. Do you know if it reduces the bundle size at all?

@mrmartineau
Copy link

Also @zenorocha do you know when this might be merged?

@realityking
Copy link
Author

realityking commented Mar 20, 2018

@mrmartineau I haven't measured it but when bundling with webpack it should be a bit smaller because it's just one CJS module, not two.

I'll actually remove the ESM version from the package.json. Compatability is still a bit funky, I now actually think it shouldn't be published right now. (<-- this is done)

@realityking
Copy link
Author

realityking commented Jun 16, 2018

@zenorocha Could you have a look at this one? Thanks.

@realityking realityking changed the title Switch bundling to rollup.js and add ESM version. Switch bundling to rollup.js Jun 16, 2018
@realityking
Copy link
Author

@zenorocha I just updated this PR with the newest rollup version. Any chance you could review it? 🙏

@fregante
Copy link
Contributor

I like this but instead of having two files it can just output the umd version. It's already universal. Webpack's bundle is always bigger, especially when not minified

@realityking
Copy link
Author

@bfred-it

I like this but instead of having two files it can just output the umd version. It's already universal.
Webpack's bundle is always bigger, especially when not minified

You mean drop the CJS version? The CJS version is smaller and should haver a (slightly) lower startup time (due to less function invocations. Obviously a drop in the bucket for any given modules, but we have dozens of dependencies.

I think it's best to give consumers the most efficient code possible - especially when it's as seamless as with npm packages.

@realityking
Copy link
Author

Hi @zenorocha,

I updated this PR to be free of merge conflicts and deprecation warnings. I also updated all the newly added dependencies to their newest version. It'd be great if you could review this. It'd be a nice little bundle size saver.

Note this PR includes the commit from #37.

@fregante
Copy link
Contributor

It’s been 3 years without PRs merged. If you want to save space and avoid bugs, use the forked version: https://github.com/fregante/delegate-it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants