rtfeldman / seamless-immutable

Immutable data structures for JavaScript which are backwards-compatible with normal JS Arrays and Objects.
BSD 3-Clause "New" or "Revised" License
5.37k stars 195 forks source link

Immutable is not Immutable #100

Closed danfma closed 8 years ago

danfma commented 8 years ago

Simple test:


var p = { name: "Daniel" };
var i = Immutable(p);

i.name = "Daniel Alves"; // should not be changed but has changed at all

\ Probably reason**

The Object.freeze is inside the node checking if.

function makeImmutable(obj, bannedMethods) {
    // Tag it so we can quickly tell it's immutable later.
    addImmutabilityTag(obj);

    if (process.env.NODE_ENV !== "production") {
      // Make all mutating methods throw exceptions.
      for (var index in bannedMethods) {
        if (bannedMethods.hasOwnProperty(index)) {
          banProperty(obj, bannedMethods[index]);
        }
      }

      // Freeze it and return it.
      Object.freeze(obj);
    }
danfma commented 8 years ago

I made a pull request to this.

smashercosmo commented 8 years ago

But I think it's intentional, isn't it? Objects are frozen only in development mode. Because in production it will result in performance issues.

camjc commented 8 years ago

@smashercosmo I think the issue is that it is not freezing in non-node environments (like a browser). @danfma Is that right?

crudh commented 8 years ago

From the project description:

In the development build, objects are frozen. (Note that Safari is relatively slow to iterate over frozen objects.) The development build also overrides unsupported methods (methods that ordinarily mutate the underlying data structure) to throw helpful exceptions.

The production (minified) build does neither of these, which significantly improves performance.

So if you use the main /src/seamless-immutable.js it just works and respects your NODE_ENV setting if you run it in node. If you run it in a browser you can either use the same file if you have a build environment where you have configured it or you can use the /seamless-immutable.development.js or /seamless-immutable.production.js directly which requires no setup of NODE_ENV.

crudh commented 8 years ago

If you want to use the default /src/seamless-immutable.js and configure your build to set the correct mode then do as they suggest in React: https://facebook.github.io/react/downloads.html#npm (check the note for npm). It works in the same way for seamless-immutable.

rtfeldman commented 8 years ago

Yep, this is working as intended. Thanks to all who responded explaining why this works the way it does!