1054426 - Update Object.assign implementation to latest spec

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

At the last TC39 meeting, it was decided that Object.assign should not throw for undefined and null source values [1].

We (Facebook) currently polyfill Object.assign and depend on this behavior, however with native Object.assign landing recently, we're now hitting uncaught errors because of null/undefined being used as sources. We'll likely update our polyfill to catch the currently wrong native impl and handle it, but it would be good to get this updated as soon as possible.

[1] https://github.com/rwaldron/tc39-notes/blob/master/es6/2014-07/jul-29.md#revisit-objectassign

Comment 1

[Sebastian Markbåge]

We won't patch this for a while though so Facebook.com will continue to be broken in Firefox Nightly until this is fixed.

Comment 2

[LegNeato]

Probably caused by http://hg.mozilla.org/mozilla-central/rev/cd27648dc4a7

Comment 3

[Jeff Walden [:Waldo]]

So is this "latest spec" that's not even published yet just saying do nothing at all for null/undefined sources?  Or what, exactly?  Meeting minutes are entirely imprecise about what exactly the change is here.

And, um, why is Facebook depending on/polyfilling still-being-standardized methods that (as far as I know) not a single engine yet implements?  In what world can this possibly help specifications receive the experimentation necessary to select the generally best API?

Comment 4

[Sebastian Markbåge]

Yes. It will say to do nothing at all for null/undefined sources. null/undefined target still throws.

We (Facebook) try to stay on the latest up-to-date polyfills so that we can test them out in real world scenarios. That helps these specs receive experimentation in a large code base. It is how we found the need for this spec change.

This also ensures that we notice when implementations diverge from the latest agreed upon changes to the spec. If we found that we've diverged in our polyfills or reliance, then we fix it on our side.

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Jeff landed a patch in https://hg.mozilla.org/integration/mozilla-inbound/rev/a203ef1e2b8f
And I cherry-picked it over to m-c in https://hg.mozilla.org/mozilla-central/rev/ffdd1a398105 to get it into tonight's Nightly.

Comment 7

[Jeff Walden [:Waldo]]

Whatever you think the value of polyfilling not-yet-fully-drafted methods is, it's really really really not worth it.  You have to deal with incremental stages of specification.  You have to deal with spec changes as they happen (and they happen).  You have to get the polyfill *right* -- which, I would note, you do not, in that comparing target or source to null using == is not sufficient to handle null/undefined values.  You make it more difficult for browsers to implement draft-spec methods, because they have to deal with the semantic minefield you're imposing on them.  (Whether you update to spec or not, you're effectively legacy content that has to be evangelized to newer semantics, if the browser moves faster than you do.)

If you really want to play this dodgy game, just overwrite the methods entirely.  If you're keeping an eye on spec changes anyway, that lets you experiment and fool around, without the possibility of making browsers implementing the spec break Facebook.

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a203ef1e2b8f

Comment 12

[Allen Wirfs-Brock]

(In reply to Sebastian Markbåge from comment #4)
> Yes. It will say to do nothing at all for null/undefined sources.
> null/undefined target still throws.
> 
Correct, this change has been incorporated into my working draft as per https://bugs.ecmascript.org/show_bug.cgi?id=3096 

Spec. Rev27 which includes the change should be release soon.

Comment 13

[Florian Scholz (Open Web Docs)]

Mentioned on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign