-
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
fix(dom.js): added support for nextjs #97
Conversation
Added support for nextjs by fixing error "document is not defined" when used in a nextjs application fix #80
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Kindly review @shshaw & @notoriousb1t . |
src/utils/dom.js
Outdated
@@ -61,5 +61,5 @@ export function $(e, parent) { | |||
? // a single element is wrapped in an array | |||
[e] | |||
: // selector and NodeList are converted to Element[] | |||
[].slice.call(e[0].nodeName ? e : (parent || root).querySelectorAll(e)); | |||
[].slice.call(e[0].nodeName ? e : document.querySelectorAll(e)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this fix "Document not defined?" This is also a departure from the expected logic that finds the elements inside a parent if one is provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, this is the only logic change. If there's something I missed, do let me know. In the future, try to scope PRs to more focused changes so real changes are more apparent.
Added "(parent || document)" in the place of "document" to find the element inside a parent if one is provided
Kindly resolve the "Document not defined" error as it's a big issue with using your package with nextjs. Works on my end as implemented in my latest commit. |
Added support for nextjs by fixing error "document is not defined" when used in a nextjs application
Also added build support for ES6+ using terser
fix #80