omniscientjs / immstruct

Immutable data structures with history for top-to-bottom properties in component based libraries like React. Based on Immutable.js
374 stars 21 forks source link

IE8 not supported in 1.4.0 because use of Object.defineProperty #51

Closed amccloud closed 9 years ago

amccloud commented 9 years ago

https://github.com/omniscientjs/immstruct/blob/master/index.js#L169-L173

Is exporting the instances property necessary or can we do it another way? I can't seem to find any documentation to support require('immstruct').instances

mikaelbr commented 9 years ago

This is mostly due to backwards compatibility. We might deprecate the property accessor and have a get-function instead, but that would be in a major release. One thing you could do until then is to add a kind of "polyfill" to defineProperty, where you just set a noop to defineProperty, making the code work but not being able to access instances.

You can see documentation of the instances property here: http://omniscientjs.github.io/api/02-immstruct-api-reference/#properties

amccloud commented 9 years ago

@mikaelbr I actually am using es5-sham for defineProperty but it looks like the issue is with it https://github.com/es-shims/es5-shim/issues/297

dashed commented 9 years ago

@amccloud Have you tried other polyfills? https://github.com/zloirock/core-js

mikaelbr commented 9 years ago

I think this is a issue with the polyfill. But, as we have a breaking change the API for this will change. As of 2.0.0 you do:

var immstruct = require('immstruct');
var s = immstruct('key', { foo: 1 });

immstruct.instance('key');

// or 
var i2 = new immstruct.Immstruct();
var s2 = i2.get('key', { foo: 1 });
i2.instance('key');