Skip to content
This repository has been archived by the owner on Jul 13, 2020. It is now read-only.

Overloading the import's referer for passing configuration? #26

Closed
brettz9 opened this issue Aug 17, 2013 · 13 comments
Closed

Overloading the import's referer for passing configuration? #26

brettz9 opened this issue Aug 17, 2013 · 13 comments
Labels

Comments

@brettz9
Copy link
Contributor

brettz9 commented Aug 17, 2013

I just made a suggestion at https://gist.github.com/wycats/51c96e3adcdb3a68cbc3/#comment-888228 for a configure() method, but pending any answer on configuration that works across all import calls, I was wondering whether anyone knew whether the import method's referer argument is intended to be usable for containing configuration properties beyond the "name" and "address" ones?

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 17, 2013

(Sorry that this is more of a support question rather than a bug or request, but I just don't know where else to ask questions--or submit feedback for that matter.)

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 17, 2013

(For another throw-away though unrelated question: are you open to a JSLint or JSHint patch?)

@guybedford
Copy link
Member

No the referer it is just for name and address. The metadata property is for extra information output during the normalize process.

That said, the better practice is to handle this state internally anyway and not share any extra state in these objects (a personal disagreement with the spec shared with James Burke).

Can you give an example of why it might be useful? It's quite difficult to understand from the description of what you are asking.

Open to JSLint patches, but it depends what it is!

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 17, 2013

And yet another--are you open to adding more to the wrap built-ins? alert (and confirm, and prompt) also don't work there.

@guybedford
Copy link
Member

Yes the wrap is expected to grow. One small note - can you please create separate issues for separate points next time.

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 17, 2013

Sure--was just afraid of causing too many issues for you to close when I was planning on putting them as independent pull requests based on your response.

As far as configuration--I just think that a big part of the usefulness of the dynamic aspect of the module system is being able to avoid requiring the user to define global configuration objects or assign independent modules to a variable needing instantiaton (without any shareability between libraries of APIs).

For example, I'm working on a "plugin" which allows things like:

System['import'](
    ['module1', 'polyfill!Window.prototype.addEventListener'],
    function (module1) {
    }
);

...in order to autodetect the presence of the polyfill to determine whether it actually needs to load it at all.

The above works fine for polyfilling single missing items, but if the user wants to import a whole module of polyfills at once, such as all of the new methods directly on Array (Array.of, Array.from, etc.), I want to allow the following syntax (an ! at the end):

System['import'](['module1', 'polyfill!Array!'], function (module1) {});

which can be used in conjunction with configuration to indicate how to detect whether the whole polyfill file needs to be loaded or not without the user needing to indicate this within the polyfill call (e.g., to just check whether Array.of is supported in the environment, and if not, load them all).

I also want configuration to allow users to optionally map polyfill! shims to the file system in different ways (e.g., to auto-namespace so that it would look for Array.of inside Array/Array.of.js, while looking for String.contains inside of String/String.contains.js). (While they could still do something like npm/polyfill!Array.of to trigger configuration which maps the polyfill path to <baseUrl>/node_modules/-array.of/-array.of.js or to /jam, etc.)

If I could allow something like this by users:

<!-- No globals set in here by user-->
<script>
System.configure({autoNamespace: true, detect: {Array: function () {
    return !!Array.of;
}}});
</script>

<!-- No globals set in here either -->
<script src="polyfillPlugin.js"></script>

I think this has the advantage that there is no need for users to make library-specific calls like

System.import('polyfillPlugin', function (polyfill) {
    polyfill({autoNamespace: true, detect: {Array: function () {
        return !!Array.of;
    }}});
});

nor set config globals:

<script>
var polyfillConfig = {autoNamespace: true, detect: {Array: function () {
    return !!Array.of;
}}};
</script>
<script src="polyfillPlugin.js"></script>

One could I think still do everything in JS:

System.configure({autoNamespace: true, detect: {Array: function () {
    return !!Array.of;
}}});
System.import('polyfillPlugin', function () {});

...but if one wishes a different implementation, there is only need to change the single import path, and to consumers of my code, there is an enforcement of semantics such that they don't have to learn a new API to trace what is happening in my plugin as I have expressed it purely through standard methods whose semantics are well-known.

I think this is more in the philosophy of polyfills as one leverages familiar semantics to the utmost, leaving regular module usage to be used for the meat of the project. Yes, the specific configuration options are custom ones, but at least there are no custom constructors, etc., and again, other projects can more easily reimplement (or wrap my own calls to System.fetch() or whatever) if not standardize by a future spec reserving the particular namespaced options.

Not sure if this makes sense...

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 17, 2013

Besides any reaction you may have on why I would like a formal configure() option in the spec (or my question at #23 (comment) ), I've now addressed the other concerns I raised in this issue by separate issues: for JSLint (issue #27) and adding alert/prompt/confirm (issue #28). Thanks for your patience...

@guybedford
Copy link
Member

That's great... I will get back on this question when I have had a little more time to consider it.

@caridy
Copy link
Contributor

caridy commented Aug 17, 2013

@brettz9 I think conditional loading should be applied at the loader level, at least that's my perception of the matter. The import method API is very clear and settle. Here are more thoughts about this from my experience with YUI Loader infrastructure, which is fairly similar:

An example first:

  • module foo imports bar.
  • module foo should be used only if addEventListener is available.
  • module baz can be a substitute of module bar if addEventListener fails, exposing the exact same API.

this is a typical case of conditional loading where a custom metadata can be plug into the loader instance to do:

a) analyze every import, when foo is requested/imported, conditions should be applied to decide whether or not it should be loaded or substituted before loading the module and the dependencies.
b) additionally, we might want loader to implement combo capabilities to load multiple modules are once when the serving infrastructure allows it.

note: conditions are, normally, feature detection routines that are cacheable and can be controlled by the loader, along with the metadata needed for it.

When looking at this from the module consumer perspective:

  • consumers of the module don't really need to deal with conditional statements definition in most of the cases. In the example above, they just want to get foo, or something similar that has the same API when they call import.
  • consumers of the module might want to force or tweak the way a module is loaded, or the conditions associated with it, but without having to change their app logic, at the end of the day, modules are all about fixed APIs. From that perspective, they just need to modify the loader or potentially the meta used by the loader.

@guybedford
Copy link
Member

@brettz9 I agree with your approach to polyfills - they should be modular and easily loaded through feature detection in this way, and not need to be globally loaded.

Your approach to a polyfill detection can be solved with a custom loader, without needing any special configuration passing though.

Configuration Injection for Custom Behaviour in a Loader

You could try something like the following:

  window.MyLoader = new Loader({
    normalize: function(name, referer) {
      if (name.substr(0, 9) == 'Polyfill!') {
        // do polyfilling stuffs
        this.polyfillSettings.namespace // ...
      }
      else {
        return System.normalize(name, referer);
      }
    }
  });
  window.MyLoader.config = function(options) {
    this.polyfillSettings = options.namespace; // ...
  }

  // loads a normal module
  MyLoader.import('some-module');

  // do polyfill config
  MyLoader.config({
    namespace: ...
  });

  // loads a polyfill if necessary
  MyLoader.import('Polyfill!Array');

I think the above is what you are looking to do.

That said the above is not necessarily a good thing to do... loaders should be written carefully to ensure they are solving more general problems, and not the specific use cases like this. There is no reason not to experiment with one though either.

Conditional Loading Techniques

What this problem does come down to, as @Cassidy describes exactly, is how to handle conditional loading within a custom ES6 loader.

I like the approach of having a static include without more complicated syntax as well.

To use @Cassidy's example, let's consider that foo.js is some code that needs a Polyfill library bar.js. This Polyfill implementation can either be empty on modern browsers, or be different for IE and Mobile say.

A solution I have been playing around with is to implement conditional loading at the custom loader level:

foo.js

  import bar from 'bar';
  // do stuff with bar

bar.js then provides a 'conditional load implementation':

  export * from 'ie ? ./bar-ie, mobile ? ./bar-mobile';

with bar-ie.js and bar-mobile.js providing the relevant implementations.

The environment variables ie and mobile etc would then be configured as globals of the loader.

When in a build mode, the custom loader can then provide output for bar.js that matches the environment build if necessary allowing build-time substitution.

Thanks... it is really great to discuss this. I'm tracking work around this on the JSPM loader here - systemjs/systemjs#9.

@brettz9
Copy link
Contributor Author

brettz9 commented Aug 20, 2013

Hi guys,

Sorry to be getting back to you late on this.

@caridy: Yes, I agree that consumers shouldn't have to deal with conditional statements themselves if possible. The reason I asked about overloading the import method was because I didn't see any other way for configuration options to be passed such that my plug-in could avoid defining any variables; it may seem silly, but it just struck me as more aesthetically pleasing to have a plugin which was meant to support global polyfills (of standard language features which are intended to be unobtrusive to the degree that they, unlike shims, do not even require the creation of local variables to which shim module imports would be assigned) itself operate in a wholly self-effacing manner. :)

But my question on import is not really something I needed for my implementation because you're right even for my need--consumers shouldn't (usually) have to deal with conditionals. I think I ended up making this report just to confirm my hunch that the module loader's current ability to allow such passing of configuration through the import in the current implementation was not deliberate.

I would expect that conditional loading of entirely different modules such as you describe (as opposed to conditional loading of one module (to set a global) or do nothing) would typically be for the sake of providing hardware-tweaked or fallback behavior to compensate for a lack of full functionality. Otherwise, I don't think it is good practice (at least I think it would be a waste of time) to design separate modules with the same functionality when an older system could instead be brought up to speed with a well-written polyfill loaded only for such a standard-deficient system.

Even within a module, I think it is good, wherever possible, for loads to be declarative as well (e.g., as with my specifying a desired polyfill rather than having to add the checks inline). Admittedly, whenever you abstract anything (unless it is itself just a polyfill with familiar semantics), you force readers of your code to have to become familiar with some more code usually that they have to find in another file elsewhere. But as with the reason-for-being of modules, this trade-off is often balanced with the desirability of custom code which makes things cleaner than what is currently possible through standards (or convention).

@guybedford: Despite the solution you provided as indeed being an option for me and despite my not having interest in passing in configuration at the import-level, I still think it would be nice if my suggestion for a System.configure() method could be adopted for initializing global System.import configuration. If state is going to be allowed within loaders, it may as well be all the way, no? :) It is similar to how JavaScript offers the ability to override native prototypes. It may not always be a good idea when something can conflict with others, but some have arguably put this to some good use (e.g., Prototype.js).

But to clarify on one point. Yes, your (draft-standard-supporting) example, besides its need for declaring a variable, was along the lines of what I was looking to do--with the possible exception that I'd probably almost always want to execute the configuration of the loader before importing any modules, as I'd hope that all nested modules using the "polyfill!" syntax would abide by the same conventions and allow imported modules to use the syntax as well.

I believe your description of there being a sometime need for conditional loading is indeed the case, and evokes thoughts of the popular Modernizr. However, I still think that it is cleaner, especially given the frequent need of polyfills (at least when one seeks to make the core of one's code more succinct and expressive by taking advantage of the latest language innovations) to handle polyfills in a special way.

If people have to define full conditions each time they want to state that they require Array.prototype.map (and possibly need to list it again in their package.json file in the case of polyfills of upcoming language features within Node), for example, it just starts looking a little ugly and implementation-detailed rather than declarative and brainless as I think the module loading process should be, at least for common use cases like polyfills. And unlike for say loads dependent on whether something is a mobile device or a particular mobile device (or the less-than-desirable user agent detection as opposed to feature detection), the polyfills I am speaking of are, I think, far less likely to change over time and thus not as likely be a fad that later becomes obscure by adding a layer of abstraction that your conditional loader would otherwise beat in transparency. Also, the abstraction my plugin concept offers is that one can also build on it to support different package managers into the future and not worry about their location (along the lines of my npm/polyfill! example--if a new pm comes along, the polyfill can detect the prefix and map the path to the conventionally required file path--as is important in browsers where package.json detection is not desirable).

(Even Modernizr which started the popular Polyfills wiki at https://github.com/Modernizr/Modernizr/wiki/HTML5-Cross-browser-Polyfills and which uses conditional loading along the lines of the loading you describe were also very positive to my listing such declaratively-stated, plugin-dependent but modular polyfills as I mention to their wiki (as I have not gotten around to doing yet but hope to do once I may get some sample polyfills adapted and working with the ES6 module loader plugin (and adapting and renaming my equivalent "shim" plugin for RequireJS), and once these samples are available for easy installation and loading-without-added-configuration from the likes of npm).)

As far as your statement about loaders being written to solve more general problems, I think that as long as it is an unobtrusive plug-in implementation (similar to good Node.js middleware) one need not worry about meeting everyone's needs. Indeed, I think it is nice if plug-ins can themselves be modular. :)

But again, I do see a place for explicitly conditional loading as in your JSPM loader.

Btw, this may be premature, as I may need another few days or so on it, but what do you think about setting up your wiki here for a page on ES6 loaders or plugins (where I define a plugin as one that modifies the global loading behavior as opposed to using the Loader constructor)? Of course, they shouldn't need to be specific to your implementation, but might be nice to have a place to discover them.

@guybedford
Copy link
Member

Yes there is the potential for the loader itself to take a role in automatically polyfilling ES features in older environments. But I don't think a configuration option solves it, and even if I did, I don't have the power to change the specification.

I considered here the possibility of entirely polyfilling ES6 -> ES5: #31.

Effectively we consider an environment as being ES5, ES6, or ES7. Then we consider a script as being ES5, ES6, or ES7. If the script is of a higher environment, we load a suitable translation polyfill.

This is certainly something that can be added to the System loader potentially as a solution to this polyfill problem.

More fine-grained conditionals would also be supported as discussed above, although I do agree this type of conditional is more useful for say mobile v desktop script loading.

@guybedford
Copy link
Member

Closing this, but happy to continue discussion any time!

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

No branches or pull requests

3 participants