Skip to content
This repository has been archived by the owner on Nov 6, 2024. It is now read-only.

enable "promisify" / "denodify" of function that loads the JSAPI #30

Closed
tomwayson opened this issue Sep 4, 2017 · 3 comments
Closed

Comments

@tomwayson
Copy link
Member

tomwayson commented Sep 4, 2017

As a stopgap to #8 we could make it easier for consumers and the wrapper libraries to promisify / denodify the bootstrap function by switching the order of the arguments so that the callback comes last.

Instead of making a breaking change for this, I propose deprecating bootstrap() in favor of a new function w/ the correct argument order. I've never been a huge fan of the name "bootstrap" anyway. I'm leaning toward loadScript() but am open to suggestion.

I also propose that we make the changes suggested in #28 in this new function since those might be considered breaking. As long as we're making what would otherwise be breaking changes in a new function called loadScript(), I suggest that callback for this new function always return the HTMLScriptElement instead of window.require in order to address the issue I brought up in #28 (comment). Or maybe it would be more useful to return the verson of the JSAPI that was loaded?

My goal for this is to reduce these ~50 lines in ember-esri-loader to:

const loadScript = RSVP.denodeify(esriLoader.loadScript);
this._loadPromise = loadScript(options)
.then(version => {
  // update the isLoaded computed property
  this.notifyPropertyChange('isLoaded');
  // TODO: remove success: true next major version bump?
  return { success: true, version };
});
@tomwayson
Copy link
Member Author

tomwayson commented Sep 4, 2017

While we're at it, let's take an optional url instead of options, so:

// default, latest JSAPI 4.x
esriLoader.loadScript(callback);
// or use a specific version/location of JSAPI
esriLoader.loadScript('https://js.arcgis.com/3.21', callback);

Originally I used options b/c I thought we might want to let users set other attributes like defer on the script tag, but no one seems to be asking for that (and TBH, I don't think that particular use case even makes sense). If later on we decide that we need options we can use TS union types to allow passing in a string (url) or object (options).

@tomwayson
Copy link
Member Author

Well, #39 finally introduces a use case for sticking with options, and I plan to implement that before this.

So what should the API be for the arguments that come before the callback? Should it continue to just take an optional options hash? The problem w/ that is that most often the only "option" consumers want to pass is the url, so I'd rather have some way for consumers to do just that. I like how esri-promise takes two optional arguments: a url string and a dojoConfig hash, but I think I prefer to take a single optional argument that if is a string it is just a shortcut for options.url, but maybe that's too trixy.

Finally, I think promisify/denodify will work w/ all those optional arguments, but I don't want to spend a lot of time testing that, so I may go straight to #8 instead of this once we decide on the API.

@tomwayson
Copy link
Member Author

closing this in favor of #8 which should be resolved in #48

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

No branches or pull requests

1 participant