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

Provide a way to synchronously use modules that you know are loaded #71

Open
tomwayson opened this Issue Jan 4, 2018 · 12 comments

Comments

Projects
None yet
6 participants
@tomwayson
Member

tomwayson commented Jan 4, 2018

One of the few drawbacks of the esri-loader approach is that just getting a reference to JSAPI modules is always asynchronous, even if we know the module has already been loaded (i.e. was included in the Dojo build layer). A good example is esri/config - you may want to set corsEnabledServers on that before loading a map. Currently you'd have to call loadModules(['esri/config']).then(esriConfig => {}) to do that.

However, if you pass a single module id as a string (instead of an array of module ids) to Dojo's require() it will return that module synchronously as long as it has already been loaded. See the "alternative require() syntax" in the Dojo loader docs. So, this library could expose a function that wraps a call to require(moduleId) using that syntax like:

import { getModule } from 'esri-loader';
const esriConfig = getModule('esri/config');

This would be a naive pass through, and would not attempt to verify that the script has loaded nor that the module exists nor fallback to loading it asynchronously, etc. It would just throw an error in those cases.

If you think this would be a useful feature, add a 👍

Conversely if you think this is not useful, add a 👎

@jpeterson

This comment has been minimized.

Member

jpeterson commented Jan 4, 2018

This would be an excellent feature as long as folks know what they're doing (read: document/warn about it well).

@jpeterson

This comment has been minimized.

Member

jpeterson commented Jan 4, 2018

if you pass a single module id as a string (instead of an array of module ids) to Dojo's require() it will return that module synchronously as long as it has already been loaded (i.e. included in the build layer).

I suspect anyone creating their own build is comfortable enough to know what it contains. But is there any documentation that would help developers know which modules are included in the build layer of the CDN hosted version of the JSAPI?

cc @lheberlie @odoe @andygup

@davetimmins

This comment has been minimized.

Contributor

davetimmins commented Jan 4, 2018

Does this mean that if someone tries to use that function whilst the API is still loading it will fail? The config example is something I would associate with someone doing if they are preloading the API, so that means they'd still need to check it was loaded before doing the getModule call. Even if they have the main map / API interaction in a sub route you can usually load directly (deep link) into that path so it would still be an issue. Just raising this as I could see it causing you headaches from a support POV, but if I've misunderstood please disregard.

Only other thing I'd add is that if you want to work in a more synchronous manner and are using something like the webpack then you can use the ignore externals technique and you get a similar end result.

@tomwayson

This comment has been minimized.

Member

tomwayson commented Jan 4, 2018

Thanks @davetimmins

Does this mean that if someone tries to use that function whilst the API is still loading it will fail?

Yes.

The config example is something I would associate with someone doing if they are preloading the API

Yes, and since 1.6 I suppose you can actually use loadModules() to preload the API, so something like this:

// preload the JSAPI
loadModules('esri/config')
.then([esriConfig] => {
  // use esriConfig to configure CORS etc
});

Then in a sumbodule somewhere you can just call:

loadModules(['esri/views/MapView', 'esri/WebMap'])
.then(([MapView, WebMap]) => {
  // this block should not execute until after the config was set... I think... I'd have to double check...
});

So config might not be the best example - In my own code, I've noticed needing references to classes like Extent outside of what would ordinarily be an async operation (like creating a map and/or loading it's layers). Usually however, it's to create an instance that will be used later in an async operation, so in those cases I just move the new Extent() inside of loadModules() inside the function that performs the async operation and pass in a POJO extent to that function. It's a minor inconvenience, but I've heard it as a gripe from a few others. No one has opened an issue yet though.

Just raising this as I could see it causing you headaches from a support POV

Yes, I completely agree that is likely going to be the case, which is why I'd want to see a lot of 👍 on this before taking it on.

if you want to work in a more synchronous manner and are using something like the webpack then you can use the ignore externals technique and you get a similar end result.

True, but part of my secret agenda w/ this issue is to address the one "advantage" that technique has over esri-loader. At this point, I think there are so many pros on the esri-loader side, that I'm going to start actively discouraging people to use the "ignore externals" approach. import map from 'esri/map' is nice, but it's misleading and comes at the cost of not being able to:

  • lazy load or pre-load the JSAPI
  • use in isomorphic/universal/server-rendered apps
  • use w/ whatever cli/boilerplate your choose w/o knowing how to tweak the bundler config
  • hit lighthouse initial load targets for PWA apps

Also feel free to 👎 if you don't think it's worthwhile. I have half a mind to do that myself. 😜

@davetimmins

This comment has been minimized.

Contributor

davetimmins commented Jan 5, 2018

Ok cool, downvoted :) Yea I never use the ignore externals technique, I find it too cumbersome with the setup and messing around with the script injection etc.

@odoe

This comment has been minimized.

Collaborator

odoe commented Jan 5, 2018

@jpeterson to check what's in the CDN build layers, look at the Layer Contents section of the build reports.
https://js.arcgis.com/4.6/build-report.txt

@jpeterson

This comment has been minimized.

Member

jpeterson commented Jan 5, 2018

I would agree that the example of esri/config is not the most compelling here.

For what its worth, my annoyances with async-ing everything include:

  • deserializing JSON objects back into JSAPI primitives
  • working with Extent, webMercatorUtils, and geometryEngine
@andygup

This comment has been minimized.

Member

andygup commented Jan 8, 2018

I'm going to give this a 👍 since I see some benefits and don't really see any harm in having the option ( as long as it's documented well 😃 ). Some of the strongest use cases for optional synchronous module load capability come from apps that aren't map-centric. For some apps I've worked on this capability comes into play after the app has loaded and you don't have to worry so much about taking a synchronous request performance hit in the UI.

import map from 'esri/map' is nice, but it's misleading and comes at the cost of not being able to:

OMG, Totally agree!

@tomwayson

This comment has been minimized.

Member

tomwayson commented Apr 14, 2018

Another way to have synchronous access to modules but w/o the the uncertainty of the above solution is to follow the pattern below.

Let's say you need to create a map in one component, and then in another component create a legend. In this scenario, you need to create the map first, then create the legend only once you have a reference to the map. However, it is unlikely that you have both references to both DOM nodes at the time you want to start creating the map (for example if the legend component is a child of the map component and it's render lifecycle hasn't started yet).

One way to do this is in a service (or any singleton module in your app) add functions like:

  newMap (elem, options) {
    // load BOTH the map AND legend modules
    // even though we're not using the Legend at this time
    return loadModules(['esri/map', 'esri/dijit/Legend'])
    .then(([Map, Legend]) => {
	  if (!this._Legend) {
        // keep a reference to the Legend class so that legends can now be created synchronously
        this._Legend = Legend;
      }
      // create and return the map
      return new Map(elem, options);
    });
  },
  // synchronous function to create a new legend
  // will throw an error if newMap() has not already been called and returned
  newLegend (params, srcNodeRef) {
    if (this._Legend) {
      return this._Legend(params, srcNodeRef);
    } else {
      throw new Error('You must have loaded a map before creating a legend.');
    }
  }

Then once the map component's DOM has loaded (like ngInit() or componentDidMount()) you can run something like:

mapService.newMap(elemRef, options).then(map => {
  // TODO: somehow signal to the legend component that the map has loaded
  // i.e. pass it down as a prop, etc
});

Then in the legend component, whenever you receive a new map instance (i.e. via prop, etc), you can run something like:

this.legend = mapService.newLegend({ map: this.map }, elemrRef);
this.legend.startup();

While that is a little complicated, what I like about it is that the developer is in complete control over which modules can be made available synchronously, so there's no mystery about why attempts to load modules synchronously might fail (either b/c the JSAPI hasn't been loaded, or the module is not one of the ones that can be loaded synchronously).

@tomwayson

This comment has been minimized.

Member

tomwayson commented Apr 14, 2018

A variation of the above pattern can be seen in this ember app, where there is a map service w/ a newMap() function that is guaranteed to be called before it's refreshGraphics() function. So the _newGraphic() function is dynamically created when the map is loaded.

@alcis-dev

This comment has been minimized.

alcis-dev commented Jul 9, 2018

#122 suggests something similar to this - and is working in prod.

@tomwayson

This comment has been minimized.

Member

tomwayson commented Jul 17, 2018

To clarify, #122 still uses loadModules() which is async. Yes, once loaded, the global modules are available synchronously. This is similar to the singleton solution I mention above, however, the singleton's functions like newLegengend() provide an opportunity to throw a more meaningful error if something tried to use the module before it is loaded. I guess you could have global functions that do the same, but you know, globals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment