-
Notifications
You must be signed in to change notification settings - Fork 20
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
Cleanup main.ts, exchange content with micro frontend #133
base: master
Are you sure you want to change the base?
Conversation
@@ -12,36 +12,11 @@ const vm = require('vm'); | |||
|
|||
const setupResourceLoader = (callback: Function) => { | |||
|
|||
const cssHandler = { |
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.
If we remove these handlers, it will not work by default anymore, right?
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 have the default handlers in the loader
https://github.com/biotope/biotope-resource-loader/blob/master/src/index.ts
thiss handlers only overwrite the default once, isnt that the case?
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.
Not if we use the class notation, currently we have an iffe version and a class version in the resourceloader. The recommended way is to use the class notation as we do here. There are no default handlers in there.
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.
Ok than i propose to change that in the resource-loader, i like to have defaults (which can be overwritten) makes it much more easy to use.
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.
@SheepFromHeaven we recommend the class version in the docs: https://resource-loader.biotope.sh/#/usage but they are not saying that you need to define a handler it says do const resourceLoader = new ResourceLoader(); and set data-resource attrs, thats it and it should work
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.
@timomayer agree to have defaults in the resource loader :)
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.
Isn't this fall on a race condition again? The init passed as a parameter on setupResourceLoader(init) should've been a callback function to force javascript sequential execution on the Resource Loader before the init function.
No description provided.