Core: Adding a method to Object.prototype causes error on ports

Created on 12 Sep 2016 · 3Comments · Source: elm/core

Whenever you try to add any functionality to Object.prototype, Elm ports throws an error and stops working.

This is a small reproducible example:

import Html
import Html.App

main = Html.App.beginnerProgram { model = model, view = view, update = update }

model = 0

type Msg = Nop
update msg model =
  case msg of
    Nop ->
      model

view model =
  Html.div [] []

Run:

$ elm make src/Main.elm --output=index.js

index.html file:

<html>
  <body>
    <div>
      <script src="index.js"></script>
      <script>
        var app = Elm.Main.fullscreen();     // embed or fullscreen are just the same
        Object.prototype.whatever = function () {};  // <--- this is the problem
      </script>
    </div>
  </body>
</html>

Served with:

$ python -m SimpleHTTPServer 3000

It says:

index.js:2800 Uncaught TypeError: Cannot read property 'push' of undefined

And if the Object.prototype.whatever line gets commented, everything goes back to working normally.

After the error, ports.send stop working.

Chrome 52
macOS El Capitan
Elm 0.17.1

bug

Source

EuAndreh

Most helpful comment

Some investigation notes:

Adding a property like this to Object.prototype is going to create a property on all objects that isn't an own property but is still enumerable. As a result, any time for .. in is used, including at the location of this error (https://github.com/elm-lang/core/blob/master/src/Native/Platform.js#L342), an enumerable property on Object.prototype is going to cause problems.

If this is something we want to fix here are a couple options with different tradeoffs:

If an object in core is intended to be used as a dictionary or bag of some kind, create it with Object.create(null). This creates a prototype-less object with no properties but its own.
- Pro: no added performance cost during iteration, as the for .. ins can stay how they are
- Con: No support for IE8
Guard every iteration of a for .. in loop with hasOwnProperty().
- Pro: perfect browser support
- Con: has to be added in a lot of places and adds an additional function call to every iteration over an object's properties

lukewestby on 12 Sep 2016

👍2

All 3 comments

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot on 12 Sep 2016

Some investigation notes:

If this is something we want to fix here are a couple options with different tradeoffs:

If an object in core is intended to be used as a dictionary or bag of some kind, create it with Object.create(null). This creates a prototype-less object with no properties but its own.
- Pro: no added performance cost during iteration, as the for .. ins can stay how they are
- Con: No support for IE8
Guard every iteration of a for .. in loop with hasOwnProperty().
- Pro: perfect browser support
- Con: has to be added in a lot of places and adds an additional function call to every iteration over an object's properties