newoga / babel-plugin-transform-replace-object-assign

Allows you to provide custom implementation of Object.assign in babel builds
MIT License
12 stars 2 forks source link

Does not replace babel helper #2

Open ljharb opened 7 years ago

ljharb commented 7 years ago

Even using this plugin, I'm still seeing this code in my build output:

var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };

Could this plugin also replace this with var _extends = require('my-configured-module')?

newoga commented 7 years ago

Hey @ljharb, you're correct that this plugin does not replace the _extends function provided by babel-helpers. It's been awhile since I've gone through babel internals, but I'd imagine that assuming all of the user's Object.assign calls are replaced by another implementation by this plugin, that _extends would only be used for things like object rest spread.

While I imagine replacing the _extends implementation would be doable, I would be curious to hear what your motivation is. The original reason I created this project is no longer relevant and I'm finding less reasons for this plugin to exist, but I'm open to keeping it active if the community still has a need for it!

ljharb commented 7 years ago

Object rest/spread is exactly my use case - specifically, in published modules, I never want Object.assign to appear, and I want to use my object.assign shim/polyfill instead - and additionally, I'd like to keep my babel output size minimal.

I think it's hugely relevant, and I actually want this for every static builtin method :-)

newoga commented 7 years ago

Cool! Yeah I was wondering about object rest/spread as I wrote my reply. I just updated the README.md provide some additional context as to why I originally created this plugin.

Nevertheless, I understand that there could still be some valid use cases for this project, especially for library authors (even if they don't currently apply to me). I don't have immediate bandwidth to take this on now, mostly because it's been sometime since I've worked with babel plugin development and I'd have to re-familiarize myself.

I'd be more than happy to accept a MR if you wanted to give it a stab yourself.

Side note: Are you familiar with this plugin: https://babeljs.io/docs/plugins/external-helpers. That plugin wouldn't replace the Object.assign implementation, but it would remove the declaration from the top of each file and replace it with an import (similar to how this project works).

ljharb commented 7 years ago

Yes, but external helpers doesn't let me omit unused helpers, and doesn't work at module level, only at app level. Also, my polyfills are better than babel's :-)

newoga commented 7 years ago

Yes, but external helpers doesn't let me omit unused helpers, and doesn't work at module level, only at app level. Also, my polyfills are better than babel's :-)

@ljharb Fair enough, I can't argue with that 😄

Let me know if you want to give this a try, otherwise it might wait until I have a weekend to tinker with babel again.

Thanks for your interest and all of your work in open source! 👍

ljharb commented 7 years ago

If I have time in the meantime I will! If not, then I'll wait til you do :-D

ljharb commented 6 years ago

@newoga any update? :-D

newoga commented 6 years ago

@ljharb Not yet! 😄 Unfortunately my primary work has been taking most of my time. Let me know if you want to take a stab!

jaydenseric commented 6 years ago

@ljharb it's possible to opt-in to using the built-in Object.assign when configuring most Babel plugins and presets, mitigating the need for this plugin to replace helpers. Then you can use this plugin last to replace Object.assign everywhere with your chosen lightweight implementation. For React projects, it is a no brainer to use object-assign as it is already a React dependency, resulting in pretty juicy bundle savings.

I rewrote this plugin for Babel v7 compatibility and made object-assign the default implementation to use, released in v2.0.0-beta.0.

After spending a few days optimizing graphql-react I've managed to halve the bundle size: https://bundlephobia.com/result?p=graphql-react@1.0.0-alpha.2 (compare with v1.0.0-alpha.1). You can see working Babel config here. Some gotchas:

ljharb commented 6 years ago

@jaydenseric I prefer object.assign which is (historically, at least, I'm not sure about currently) more spec-compliant than object-assign; thanks for the config links, I'll take a look. However, I don't think it's very ergonomic to have to jump through hoops to configure something at the app level that this plugin could tackle itself :-)

newoga commented 6 years ago

@ljharb @jaydenseric This issue for babel v7 seems to have some relevance: https://github.com/babel/babel/issues/5699

If you think so, it could make sense to chime in there and add any use cases you feel have not already been covered. Having this issue (and project as a whole) solved at the babel level seems to make sense to me.