refactor(jQlite): Add the ability to use query selector without jQuery #15986
Conversation
|
There was extensive discussion about making But these arguments don't extend to root-level selectors so I can see Thanks for the PR! |
|
@rafaelfragosom Please make sure |
|
@mgol I agree, I only want to be on the same page here. Are these changes wellcome or should I close this issue? Is Let me know so I can put some more effort on this. Thank you. |
|
@rafaelfragosom I consulted the team and we're OK with making We'll need unit tests that confirm both of those conditions are met. Would you be willing to work on that? |
src/jqLite.js
Outdated
| @@ -284,7 +284,7 @@ function JQLite(element) { | |||
| } | |||
| if (!(this instanceof JQLite)) { | |||
| if (argIsString && element.charAt(0) !== '<') { | |||
| throw jqLiteMinErr('nosel', 'Looking up elements via selectors is not supported by jqLite! See: http://docs.angularjs.org/api/angular.element'); | |||
| return new JQLite(document.querySelector(element)); | |||
There was a problem hiding this comment.
You need to use window.document. We don't assume browser globals are available globally, they have to be taken from window. You can also see that the Travis build failed because of that.
There was a problem hiding this comment.
Thanks for letting me know.
I've been busy this entire time but I'll take the time to finish the PR.
There was a problem hiding this comment.
I added the window call to it and rebased everything.
orafaelfragoso
force-pushed
the
angular-element-qs
branch
4 times, most recently
from
ff4f3a1 to
389ecf4
Compare
`angular.element` threw an error if we tried to use query selector
directly without jQuery. I thought that this was a silly little
fix for the framework.
A lot of the projects that I've worked for
the past years are only including jQuery to use query selectors on
the app (I know that are options for this), and I'm only making this PR
because I think it's unnecessary to use
`angular.element(document).find()` to accomplish such a small thing and
to get the jQlite wrapper with the goodies.
Now we can do this, without jQuery:
`angular.element('.something')`
And get the same result.
use the window to call document as instructed
orafaelfragoso
force-pushed
the
angular-element-qs
branch
from
389ecf4 to
ecb0569
Compare
| @@ -284,7 +284,7 @@ function JQLite(element) { | |||
| } | |||
| if (!(this instanceof JQLite)) { | |||
| if (argIsString && element.charAt(0) !== '<') { | |||
| throw jqLiteMinErr('nosel', 'Looking up elements via selectors is not supported by jqLite! See: http://docs.angularjs.org/api/angular.element'); | |||
| return new JQLite(window.document.querySelector(element)); | |||
There was a problem hiding this comment.
This is not a good place to add this logic as it'd mean new angular.element(selector) wouldn't work. This whole if should just be removed and the logic should me moved down.
There was a problem hiding this comment.
Also, note it should use querySelectorAll, not querySelector as we want to select all matching elements, not one.
|
@rafaelfragosom Hey, are you still interested in finishing this PR? |
|
We're now in LTS mode so no new features are accepted. Changing the milestone. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
refactor
What is the current behavior? (You can also link to an open issue here)
angular.element('.my-selector')throws an error if jQuery is not included.What is the new behavior (if this is a feature change)?
angular.element('.my-selector')now finds the element and wraps it in a jQlite (or jQuery) object.Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information:
angular.elementthrew an error if we tried to use query selectordirectly without jQuery. I thought that this was a silly little
fix for the framework.
A lot of the projects that I've worked for
the past years are only including jQuery to use query selectors on
the app (I know that are options for this), and I'm only making this PR
because I think it's unnecessary to use
angular.element(document).find()to accomplish such a small thing andto get the jQlite wrapper with the goodies.
Now we can do this, without jQuery:
angular.element('.something')And get the same result.