Skip to content

Conversation

@eminaz
Copy link
Contributor

@eminaz eminaz commented Jun 25, 2017

when selecting element by id in selectorParse, use getElementById instead of querySelector for better performance (https://jsperf.com/getelementbyid-vs-queryselector), and this way it also support id that starts with a digit (querySelector doesn’t support that)

…tead of querySelector for better performance (https://jsperf.com/getelementbyid-vs-queryselector), and this way it also support id that starts with a digit (querySelector doesn’t support that)
@dmarcos
Copy link
Member

dmarcos commented Jun 25, 2017

Thanks! Almost ready to go. There's a broken test to fix

@eminaz
Copy link
Contributor Author

eminaz commented Jun 26, 2017

Fixed.

function selectorParse (value) {
if (!value) { return null; }
if (typeof value !== 'string') { return value; }
if (value[0] === '#' && !/[.,[: ]/.test(value)) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the second check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's to check if the query is '#someId.someClass div[name="b"]' etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the regex

@ngokevin
Copy link
Member

I've updated to fix tests, added tests for selectors that were not accounted for, fixed the regex.

@ngokevin ngokevin merged commit 25e3b9a into aframevr:master Jun 26, 2017
@ngokevin
Copy link
Member

thanks much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants