Skip to content
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

reset for single bundle id, and remove its script element #85

Open
jacobg opened this issue Feb 18, 2019 · 7 comments
Open

reset for single bundle id, and remove its script element #85

jacobg opened this issue Feb 18, 2019 · 7 comments

Comments

@jacobg
Copy link

jacobg commented Feb 18, 2019

I'm implementing a wrapper for manual retries after a failed load. It could be much, much later; so it's not a use-case for the numRetries option.

In any event, I need a way to remove the bundleId from the cache, and also to remove the script element from the dom. Would this be useful to add to this library?

@amorey
Copy link
Member

amorey commented Feb 18, 2019

I realize that this isn't the most elegant solution but you could implement these features using the existing API by saving a reference to the script element in the before() method and defining the bundleId using the done() method after the script has successfully loaded. For example:

var scriptEl;

loadjs('/path/to/foo.js', {
  success: function() {loadjs.done('foo');},
  error: function() {scriptEl.parentNode.removeChild(scriptEl);},
  before: function(path, el) {scriptEl = el;}
});

loadjs.ready('foo', function() {
  // foo has loaded
});

What kind of load errors are you trying to work around?

@jacobg
Copy link
Author

jacobg commented Feb 18, 2019

Thanks @amorey. I think the case it misses though is if multiple callers try to load the same script simultaneously. Since the bundleId is not defined before the async load, then there could be simultaneous requests. My wrapper also has to deal with that by checking for loadjs.isDefined. I've included my wrapper below for reference.

This script is in a Cordova hybrid mobile app, and the app could be offline at any time. When offline the maps will be in a degraded state, but need to be able to work when online in the same application lifecycle.

googlemaps.js

import constants from '@/constants'
import loadjs from 'loadjs'
import _ from 'lodash'

const googleMapsJsApiKey = constants().googleMapsJsApiKey
const googleMapJsScriptUrl = `https://maps.googleapis.com/maps/api/js?key=${googleMapsJsApiKey}&v=quarterly`
const loadjsBundleId = 'google-maps'

function handleLoadJsResult (resolve, reject) {
  return {
    success: () => {
      if (_.get(window, 'google.maps')) return resolve(window.google)
      else reject(new Error('google.maps object not available after load'))
    },
    error: () => {
      // TODO: The implication of reset() is that it clears all bundleIds.
      // TODO: So it has unintended side effects if we ever use loadjs for
      // TODO: another script.
      loadjs.reset()
      reject(new Error('Failed to download google.maps script'))
    },
    numRetries: 1
  }
}

export function loadAsPromise () {
  if (_.get(window, 'google.maps')) return Promise.resolve(window.google)

  return new Promise((resolve, reject) => {
    if (loadjs.isDefined(loadjsBundleId)) {
      loadjs.ready(loadjsBundleId, handleLoadJsResult(resolve, reject))
    } else {
      loadjs(
        googleMapJsScriptUrl,
        loadjsBundleId,
        Object.assign({ numRetries: 1 }, handleLoadJsResult(resolve, reject))
      )
    }
  })
}

@amorey
Copy link
Member

amorey commented Feb 18, 2019

I see, thanks. Do you only need this behavior for google maps or are you using it as a general coding pattern? If it's a one-off, have you considered using your own flag variable instead of bundleIds to track when a script load has been triggered?

In terms of adding a bundleId clearing feature to loadjs, you would just have to modify the code here:
https://github.com/muicss/loadjs/blob/master/src/loadjs.js#L262-L269

@jacobg
Copy link
Author

jacobg commented Feb 18, 2019

Currently I'm just using it for google maps, so reset() won't hurt me right now. But I'm just anticipating in case there's an additional use-case. And it may be useful generally for users.

@amorey
Copy link
Member

amorey commented Feb 18, 2019

I see. Then I would just use reset() until the need arises for the extra functionality later. In order to keep this library as small as possible I've tried to keep the feature set down to the bare minimum.

@amorey
Copy link
Member

amorey commented Mar 14, 2019

@jacobg FYI - LoadJS v3.6.0 includes Promise support so you might be able to use it to simplify some of your code: #87

@jacobg
Copy link
Author

jacobg commented Mar 14, 2019

@amorey That's great! Thanks for letting me know!

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

No branches or pull requests

2 participants