-
Notifications
You must be signed in to change notification settings - Fork 68
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
Create factory shortcut throws error in IE8 #140
Comments
Yikes! I'll look into this asap. Thanks, @gehan |
Yeah was throwing me for a while! Not sure if it affects using shorter factory definitions anywhere else, but def for create! |
Silly question, but are you using cujojs/poly (or another ES5 polyfill)? I figure you are, but just want to rule out the obvious :) |
I'm using |
Ok, thanks. I'm able to reproduce the problem, but as usual with IE, tracing to find the actual cause is painful :) Working on it tho. If you come across any more info, let me know! |
…tunately, we still dont know the cause
Hey @gehan, I just pushed a potential workaround to the fix-140 branch. Could you give it a try and let me know if it works for you? This is a pretty baffling problem. While the workaround does work for me, I don't yet know why, or what the actual root cause of the problem is in IE8. For some reason trying to |
Yeah, IE is super weird. I'll give it a go! Not other wire problems... however we're trying to debug why IE8 isn't working in our site. However since a lot of things get fired off inside promises... they swallow all the exceptions even if they are unhandled. I know this is a known issue and expected behaviour but couldn't we have a debug option in when.js to expose all exceptions or at least log then anyway? |
Have you tried enabling when.js's unhandled rejection monitor? |
Oh, and if this workaround works for you, I'll include it in a wire release this week :) |
@briancavalier yes thats exactly what I was looking for - AWESOME Is that safe to use in production? Or rather I'd love to capture all unhandled exceptions and log them to the console in dev and our Sentry/exception server in prod! |
I definitely wouldn't use it as-is in production :) It's purely intended for dev/debug time, and introduces some significant performance overhead. However, the interesting thing about the monitor is that it's an API. You could write your own and when.js will call it. So, for example, you could probably create a lighter weight version that doesn't do the extended stack trace stitching, or defers collecting stacks and reporting rejections until the app is idle, etc. If you want to go in that direction, just let me know, and I'd be happy to answer questions, etc.! |
Another thing you can do is, if you're using wire as an AMD plugin, always add an AMD error handler to your top-level AMD bootstrap. For example, if you're using curl: curl(['wire!myapp/main'], function(mainContext) {
// maybe nothing here
}, function(error) {
// Report fatal errors
console.error(error);
}); If the AMD loader supports that (curl does, and I think requirejs does, too), wire will hook into it. So, anything that causes a wiring failure will bubble up to the error handler. I'm also working on a related change that will report a loud stack trace when the AMD loader doesn't support an error handler. |
Yep for wire specs I've always added the fail handler! Works great I see you've added (in master) |
Yeah, in fact, I (quite literally) just published 2.6.0 :) It does have promise.done, and it will help if we use it in some strategic spots. We probably have to give a little time for people to update before we can use it in wire. I made some other changes in wire that I'm planning to release alongside this workaround for create. |
Ha and I was literally just looking for it! I'll try now. I think |
I tried it on IE8 and works fine! |
Cool, thanks @gehan. I probably won't have time today to pull a release together, but I should be able to get it out early next week. |
Slacker! ;) What's new in 0.11 then? Relative paths for plugins in specs too?? ;) Actually that was dependant on the curljs extension loader changes right? |
Yeah, we were going to try to solve that issue by implementing extension-based loading in curl. It might be good to provide some level of functionality in wire as well, though, for people who still explicitly use plugins (or can't immediately updated their specs to rely on extension-based), or another AMD loader. I've been thinking that a simple approach might be to split the module id on '!'. If there are 2 parts, and the second part starts with './' or '../', assume it's path-like and run the same relativizing algorithm that wire uses for non-plugin module ids now, then stitch the complete id back together. @unscriptable, any thoughts on that? |
I keep thinking that the AMD loader's id mapping/transform behavior would conflict with your relativizing behavior. But it won't, afaict. All that wire.js will do is normalize the id -- and that is universal across loaders, afaik, since it's defined in the AMD and CommonJS specs. If we start interoperating with other module systems that don't normalize the same way, then we might hit a problem. tl;dr: lgtm. |
Awesome @briancavalier ! Just tried it and works like a charm ;) |
Although I guess you'd need to write something for cram to get that working right? |
This should work fine with cram.js. We're relying on the fact that most AMD tools (e.g. cram) use the same algorithm to normalize module ids. If somebody were to create a plugin that doesn't follow the standard AMD/CommonJS normalization, then we'd be causing breakage. Otherwise, it should Just Work™. :) |
That was my way of saying I tried it and it didn't work ;) I'll try again just in case! |
Make sure you try the latest dev branch. I made a few tweaks. If it doesn't work, post a test case and I'll iterate on it until it does :) |
Will do! I was trying that specific commit before :) |
Nope sorry! :(
|
Fails things like
But also things like
|
|
I def don't claim that it'll work with cram yet :) have you tried it without compiling? |
Yep - already tried before and it works fine without cram! I said above actually :) @unscriptable said it should work fine in cram though, so guess this is more directed at him! |
Oops, I missed your earlier comment about it working outside of cram, @gehan. Sorry about that! I'll dig into the cram builder soon and see if I can figure out what's going on. |
If you do this
instead of
Then IE8 throws a 'Type Error: Object expected'
The text was updated successfully, but these errors were encountered: