Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for IE<11 had to be dropped? #18

Closed
kokujin opened this issue Apr 4, 2016 · 23 comments
Closed

Support for IE<11 had to be dropped? #18

kokujin opened this issue Apr 4, 2016 · 23 comments

Comments

@kokujin
Copy link

kokujin commented Apr 4, 2016

@sylvainpolletvillard, would there still be a 1.2 version with stacked errors support? My code has been blocking on this feature for a few days now. Support for IE11 and up is not an option for me. Thanks

@kokujin kokujin changed the title Support for IE<11 had to be dropped Support for IE<11 had to be dropped? Apr 4, 2016
@sylvainpolletvillard
Copy link
Owner

The decision to drop support for IE9-10 has not been easy. I know these browsers are still required for many projects.

I spent the last week trying to fix the compatibility issues on IE9 while everything was ready and functional for the other browsers. The main issue I have with IE<11 is the lack of support of proto or Object.setPrototypeOf. In v1, it implies all kinds of hacks like Model.instanceOf or ensureProto. Also, the Object.setPrototypeOf sham was far from being spec-compliant. It had to mess up with the prototype chain and could have a negative impact on other libraries using Object.setPrototypeOf. I must confess I found these workarounds not acceptable to reach the level of quality and robustness I aim for this library. That's why the support had to be dropped. I hope you understand.

Now, considering your specific needs ; could you please elaborate on how this feature is blocking for you ? I understand that error stacking can help with debugging, but never considered this as an absolute necessity. I will continue to provide bug fixes on v1.x, but I can not promise I will add new features.

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

Thanks for the explanation. The reason why I was blocking was because I need it to display the corresponding errors in a dynamically generated UI. All errors would be reflected and displayed not just the first error.

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

hmm, my bad. It seems that the MDN polyfill is not that good

// Only works in Chrome and FireFox, does not work in IE:

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

but this might interest you

https://gist.github.com/WebReflection/5593554

@sylvainpolletvillard
Copy link
Owner

Thanks but I have already seen these links :) The first one is already used in ObjectModel: https://github.com/sylvainpolletvillard/ObjectModel/blob/master/src/helpers.js#L16

Unfortunately, both of these relies on proto which is not supported by IE<11. So in v1, I had to copy all the properties from the prototype, see :

: function(o, p){ for(var k in p){ o[k] = p[k]; } ensureProto(o, p); });

But by doing this, the prototype chain is broken. That's why we had the Model.instanceOf function in v1 : the instanceof operator could not work this way in IE<11.

@sylvainpolletvillard
Copy link
Owner

I will try to mix up the v2 code related to error stacking in v1.1.5 and see how things are going.

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

That would be great

sylvainpolletvillard added a commit that referenced this issue Apr 4, 2016
@sylvainpolletvillard
Copy link
Owner

Good news ! I started over my previous work last week and I managed to get it working on IE9, with these evolutions :

  • null is now a valid value for an optional property
  • stack errors instead of throwing the first error catched
  • added custom error collectors
  • assert API has changed and now receives an optional description
  • validate API has changed and now receives an optional error collector

You can see that most of changes in v2 are here, which means you should be able to use v2 documentation and have IE9 support.

However, the limitations explained previously are still here:

  • you still have to use Model.instanceOf instead of instanceof operator
  • Object.setPrototypeOf is overwritten with a non spec-compliant sham, which means it can behave in a bad way with other libraries using Object.setPrototypeOf
  • some new internal functions in models, like validator and unstack, are exposed while they should be private.

I published this as v1.2, which should have been the initial version number before I decided to remove the IE-specific stuff.
Unit tests passed but I did not make intensive manual testing, so please give it a try : https://github.com/sylvainpolletvillard/ObjectModel/blob/master/dist/object-model-1.2.0.zip

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

You rock @sylvainpolletvillard :)

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

Hmm, I have problems installing. The stacktrace:

npm WARN install:inherits@2.0.1 ENOENT: no such file or directory, rename '/usr/local/lib/node_modules/grunt-cli/node_modules/findup-sync/node_modules/glob/node_modules/inherits' -> '/usr/local/lib/node_modules/grunt-cli/node_modules/inherits'

I am using Node version v5.3.0 on OSX

@sylvainpolletvillard
Copy link
Owner

I'm afraid this is a problem related to your configuration. I don't have any trouble with installing v1.2 dependencies, which are the same than v2. Try to remove your local node_modules folder and uninstall/reinstall grunt-cli globally.

But this is just a warning, maybe you can ignore it.

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

Hmm, very strange, I cleared the npm cache and re-installed Gulp globally, same problem. Good news though, the 'errorCollector' works as expected, now I can assign errors to certain parts of the markup. :)

I think the error api would be much quicker to use if the errors were automatically assigned to an array attribute of Model named errors.

@sylvainpolletvillard
Copy link
Owner

Like myModelInstance.errors ? This would not work for several reasons:

  • you can't add properties to primitive models like Model(Number)
  • it would prevent having a property of your model definition named errors
  • we would have to define when this property is reset, which is not a trivial choice
  • the user would have to think about every place in his code where the model could change, to manually check for errors.

But you can always do this by yourself:

var Student = Model({
    name: String,
    course: [ "math","english","history" ],
    grade: Number
}).assert(function(stu){ return stu.grade >= 60 }, "should at least get 60 to validate semester")

Student.errors = [];
Student.errorCollector = function(errors){  Student.errors = errors; };

new Student({ name: "Joanna", course: "math", grade: 50 });

console.log("Errors for Student model:", Student.errors);

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

2 things which I noticed:

1.The version attribute in the package.json file is still pegged at 2.0.0
2. The postinstall script calls 'npm install -g grunt-cli' , which would fail on machines that need sudo to install global scripts. The install fails when it tries this and stops

Apart from that, it works. Thanks!

@sylvainpolletvillard
Copy link
Owner

Are you sure you downloaded the right file ? The package.json file contained in object-model-1.2.0.zip has the 1.2.0 version as intended.
For sudo, see : http://stackoverflow.com/questions/16724259/npm-command-sudo-or-not

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

Oh, I forgot to clarify.

'npm install objectmodel --save' causes problems. The zip file I downloaded does'nt. That would mean you might have to build 2 releases(tags), 1.2.0 and 2.0.0

@sylvainpolletvillard
Copy link
Owner

I did not publish 1.2 to npm yet. I do it right now, then you'll be able to install it with: npm install objectmodel@1.2.0 --save
edit: done

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

Great! But check up on the postinstall attribute, it seems that it needs to be done differently with libraries that are installed globally, for example , http://stackoverflow.com/questions/13784600/how-to-deploy-node-app-that-uses-grunt-to-heroku

@sylvainpolletvillard
Copy link
Owner

This stackoverflow question is about running grunt command on postinstall, not installing grunt-cli globally. You should be able to install globally every NPM module without sudo by doing this : $ sudo chown $(whoami) /usr/local/share/npm/bin

@kokujin
Copy link
Author

kokujin commented Apr 4, 2016

ah well. I hope 1.2.0 installs cleanly. I have several modules that I have installed globally (Slush, Yeoman, Ava e.t.c) I really doubt if its my setup

@sylvainpolletvillard
Copy link
Owner

If you have any other questions, we should discuss on gitter to not pollute this issue with unrelated subjects

@sylvainpolletvillard
Copy link
Owner

I think I can close this issue. Feel free to open another one in case you got trouble with 1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants