-
Notifications
You must be signed in to change notification settings - Fork 6
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
Drop jQuery #59
Drop jQuery #59
Conversation
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.
Look cool :)
Generally, I'd also focus on new implementations of MediaQuery and WindowEventResizer (https://www.npmjs.com/package/@unic/factory-breakpoint-manager) (windowEventResizer not yet implemented)
@@ -75,25 +75,24 @@ class App { | |||
return window[namespace].modules[moduleName]; | |||
} | |||
|
|||
isInitialised($element, moduleName) { | |||
isInitialised(element, moduleName) { | |||
// jQuery 3 does not allow kebab-case in data() when retrieving whole data object https://jquery.com/upgrade-guide/3.0/#breaking-change-data-names-containing-dashes |
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.
This line is unnecessary when removing jQuery :)
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.
Very good point, removed
@christiansany, absolutely, but as stated above, I wanted to keep the changes as minimal as possible so we can discuss new architectural approaches separately of dropping jQuery. |
@christiansany, I have added two basic polyfills: |
Very valid point, but let's tackle this separately
"dom-delegate": "^2.2.0", | ||
"highlight.js": "^9.12.0", | ||
"jquery": "^3.3.1", | ||
"lodash": "^4.17.10", | ||
"merge-stream": "^1.0.1", | ||
"nodelist-foreach-polyfill": "^1.2.0", |
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.
You can think about using https://github.com/WebReflection/dom4. It includes these polyfills and some more. We normally load this in a separate entry file either in a sync way using document.write
or in a ordered way async.
@@ -174,10 +174,13 @@ class SlideShow extends EstaticoModule { | |||
MediaQuery.addMQChangeListener(this.resize.bind(this), this.uuid); | |||
} | |||
|
|||
fetchSlides() { | |||
async fetchSlides() { |
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.
Why not write a module so developers don't have to this evrytime something like this:
export default async function estaticoFetch(){
if(!window.fetch){
await import('whatwg-fetch');
}
return window.fetch(...arguments);
}
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.
We needed this merged, so would you be keen on opening another PR with this feature?
Notes:
dom-delegate
to replace the previous event system (after checking out about every DOM event abstraction I could find). The main difference is probably that there is no.add.some.namespaces
thingie no more. So instead of unbinding events via namespaces we rather keep track of all listeners belonging to a module. Theevents
andmediaqueries
helpers don‘t use the delegate library in a very smart way, but I guess this code will be replaced soon anyway.fetch
on demand)Mutation|IntersectionObserver
, lazy-load complete modules etc.). @christiansany has some great ideas.Thoughts?