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

Implemented loop based on hidden candidates #78

Closed
wants to merge 1 commit into from

Conversation

elmosgot
Copy link

Now all candidates are found to be wrapped. The only thing that is affected here is the performance from avg of 5ms to an avg of 30ms, about 6 times slower.

@@ -640,7 +640,9 @@
// EventTarget is all that's required in modern chrome/opera
// EventTarget + Window + ModalWindow is all that's required in modern FF (there are a few Moz prefixed ones that we're ignoring)
// The rest is a collection of stuff for Safari and IE 11. (Again ignoring a few MS and WebKit prefixed things)
"EventTarget Window Node ApplicationCache AudioTrackList ChannelMergerNode CryptoOperation EventSource FileReader HTMLUnknownElement IDBDatabase IDBRequest IDBTransaction KeyOperation MediaController MessagePort ModalWindow Notification SVGElementInstance Screen TextTrack TextTrackCue TextTrackList WebSocket WebSocketWorker Worker XMLHttpRequest XMLHttpRequestEventTarget XMLHttpRequestUpload".replace(/\w+/g, function (global) {
var list = Object.getOwnPropertyNames( window );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should do with a check to see if Object.getOwnPropertyNames is defined (< IE9).

@jmshal
Copy link

jmshal commented Sep 10, 2014

I really like this, and based on the comment (above the previous code), it would seem that the issue of Object.getOwnPropertyNames not being available in IE8 and below, isn't a problem. Am I wrong by thinking that (as mentioned) the only browsers that require that last part are 'modern' Opera, Safari, Chrome, FF and IE11?

@elmosgot
Copy link
Author

Yeah you're right that we need to check first if getOwnPropertyNames exists to not break in old browser versions.

The part of getting nice stack traces is anyway only working on newer browsers. Otherwise we use the fallback to the previous method for the browser that have not implemented getOwnPropertyNames and do have prototype inheritance support.

But as for I read here the both features are implemented in the same releases:
http://kangax.github.io/compat-table/es5/#

@elmosgot
Copy link
Author

From firebug source I found the following check:

var isObjectPrototype = function(obj)
{
// Check if an object is "Object.prototype". This isn't as simple
// as 'obj === context.window.wrappedJSObject.Object.prototype' due
// to cross-window properties, nor just '!Object.getPrototypeOf(obj)'
// because of Object.create.
return !Object.getPrototypeOf(obj) && "hasOwnProperty" in obj;
};

We can use that to determine the availability if the object contains getPrototypeOf

@ConradIrwin
Copy link

Hey @elmosgot! Thanks for this, I love the code simplification.

Unfortunately this approach seems to reliably lock up Safari (Tested with Desktop 7.0.6, and iOS 6). The test suite just gets to test 40 and then stops on 100% CPU forever.

Out of interest how did you measure the slowdown? Using console.time() and console.timeEnd() I measured before -> after as 1ms to 2ms (in Chrome/Firefox).

I think it would be safer for now to add things that we find are missing to the list, have you run across anything in the real world?

kattrali pushed a commit that referenced this pull request Apr 22, 2021
Use app path over CWD for the default project root
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

Successfully merging this pull request may close these issues.

3 participants