Skip to content

setting loading promise on request (not on load) to avoid duplicate l… #29

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

Merged
merged 3 commits into from
Jun 26, 2019

Conversation

vgavro
Copy link
Contributor

@vgavro vgavro commented Nov 20, 2018

closes #12
Setting loading promise on request (not on load) to avoid duplicate loadings before load were finished
See #12 (comment)

PROPOSAL (backward incompatible): rename Script2.loaded to Script2.load because we also store promises which is still in progress. What do you think?

Also:

  1. fixed verbose async=false (will act as async=true previously because of isUndefined check)
  2. added events on script load and script error for usage like this:
<template>
<script2 src="myscript.js" @load="loaded" />
<template>
<script>
export default {
   methods: {
       loaded () {
          window.myscriptModule.youVeGotTheIdea()
       }
   }
}
</script>

@taoeffect
Copy link
Owner

@vgavro Thank you for the pull request! I will need some time before I can get to reviewing it however, please understand. However it is now on my TODO list and is very much appreciated!

@g3rd
Copy link

g3rd commented Jan 23, 2019

@taoeffect has this moved up on your TODO list?

@taoeffect
Copy link
Owner

@g3rd Yes, thank you for the reminder! There are still some items that require my immediate attention first. If this is an urgent issue for you, I recommend simply creating a file in your project that has all of the code from this project copy/pasted into it, along with the changes here.

This is not a project that I expect to have a rapid development cycle, so you won't be missing much, and you can always check back from time to time to see if these changes were merged back in. (Plus, you'll be slightly more secure because you won't be relying on npm! 😄)

() => this.$emit('load'),
(err) => this.$emit('error', err)
)
!this.async
Copy link
Owner

Choose a reason for hiding this comment

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

The _.isUndefined check was necessary because <script async .. means the same thing as <script async=true. So we must check for both whether it's undefined, and whether async="false" is set, per https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script

@taoeffect taoeffect merged commit bd34b11 into taoeffect:master Jun 26, 2019
@taoeffect
Copy link
Owner

I've merged this and will fix the issues I found in it. Thank you for your PR and my apologies for the time it's taken me to get back to this!

@taoeffect
Copy link
Owner

Published 2.1.0! Note that load event was renamed to loaded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using with simultaneously rendered components
3 participants