ljharb / object.assign

ES6 spec-compliant Object.assign shim. From https://github.com/es-shims/es6-shim
https://tc39.es/ecma262/#sec-object.assign
MIT License
107 stars 22 forks source link

Automatic shimming #9

Closed juandopazo closed 9 years ago

juandopazo commented 9 years ago

First of all, nice work on the CI stuff!

Could we have a distribution for the browser that automatically shims the API?

ljharb commented 9 years ago

Sure - require('object.assign').shim()

I don't want to add that automatically anywhere though - automatic anything is bad code imo; everything should be explicit.

juandopazo commented 9 years ago

The problem is that this way it's hard to use the polyfill in a project that doesn't use CommonJS modules. Do you have any advice for this case?

ljharb commented 9 years ago

Projects can't really avoid a build step anymore - I'd suggest browserify first and foremost.

juandopazo commented 9 years ago

I don´t think I follow your train of thought.

A polyflil is supposed to be a vanilla JS implementation of a native feature. It´s supposed to let us write code today as it will be written tomorrow, when the runtime has support for that particular feature. With this in mind, requiring a build step or a specific module format doesn't sound like the path forward for polyfills in the browser.

I can understand having certain reservations about modifying global objects directly. The main reason is that it may hamper the standardization process. If a polyfilll becomes popular, the spec can no longer make changes to the API. What we've been doing to cover this issue is to expose our polyfills in a different global object that is clear that it's not going to be used by a specification. For example, instead of defining the Promise global object, we define a PromisePolyfill global object so that we don't get in the way of the spec getting stability (it's pretty stable by now though).

But this isn't really needed in already standardized and stable APIs. And it can be open for discussion in the case of Object.assign.

ljharb commented 9 years ago

This module is primarily meant to be a standalone function, that can optionally be used to polyfill the environment. Everything that matters is on npm, and you generally can't use those in the browser without a build step like browserify - the days of manual script tags are over.

However, <script>var module = {exports: {}};</script><script src="object.assign.js"></script><script>module.exports.shim(); module = null;</script> should definitely work, without a build step, if that's the route you prefer.

I do want this module to be useful for all use cases. Committing built artifacts to git, or distributing them on npm, is considered a pretty bad practice. However, I could certainly add an npm script so that npm run build created a browser-specific, auto-polyfilling .js file, and then you could use that file - would that work for you?

ericf commented 9 years ago

I do want this module to be useful for all use cases. Committing built artifacts to git, or distributing them on npm, is considered a pretty bad practice. However, I could certainly add an npm script so that npm run build created a browser-specific, auto-polyfilling .js file, and then you could use that file - would that work for you?

Having a build step to generate an auto-polyfillying .js file would be a great.

But I'd also like to push back on the idea that distributing dist files to npm is a bad idea. I agree with you that publishing dist/build files to Git sucks (although it does have advantages for Bower and Raw Git). But publishing dist/build files to npm is totally acceptable and is likely the main way forward for npm to become the package manager for the web, since people will want to serve these dist/build files directly to browsers (thus replacing the need for Bower).

It would be great if this package had prepublish script to run the build to create the auto-polyfilling .js in a dist/ dir that would be in the npm package. This would not affect the CommonJS usage and would easily allow people to serve the files from this package to browsers. We've been taking this approach in all of our Format.js packages and it's been working great.

ljharb commented 9 years ago

Via browserify, there's not any need for bower anyways, ever.

You're totally right that a prepublish script would be a nonintrusive way to accomplish this goal. It wouldn't require any long term effort on my part, and would make things easier for people somehow unwilling or unable to use browserify.

I'll add that to this project to close this issue.

ljharb commented 9 years ago

Published as v1.1.1. Let me know if that works for you.

ericf commented 9 years ago

@ljharb thanks! I just tested this out and in Firefox the shim doesn't apply (since it has the built-in Object.assign), and in Chrome and Safari the shim is applied automatically — so works as expected.

ljharb commented 9 years ago

Great to hear, thanks for the comment and followup @ericf!

@juandopazo, does this meet your needs?

juandopazo commented 9 years ago

That's great! Thanks a lot!