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

Require AMD? #151

Open
montlebalm opened this issue Jan 24, 2014 · 20 comments
Open

Require AMD? #151

montlebalm opened this issue Jan 24, 2014 · 20 comments

Comments

@montlebalm
Copy link
Member

Support for AMD is currently on the roadmap for version 2. Libraries like RequireJS and Dojo encourage modular development and dependency management, both of which are keystone philosophies of F2.

Critics of AMD suggest it's simply one of many solutions to the problem. As far as I know, there are no mainstream libraries that force you to use AMD. Some, like jQuery, provide an AMD wrapper with a fallback to the traditional global.

The question is should AMD be a hard dependency of F2? Should AMD support appear in the library at all? If we support AMD, is it possible to keep 100% feature parity with the global version?

Reading:

@ilinkuo
Copy link

ilinkuo commented Jan 24, 2014

I feel the above links are a little old and that the following links more accurately reflect the current consensus.

More links:

If F2 only wants to support one loader, then it ought to be AMD.

If F2 wants to support multiple loaders, then do it the lodash way by supporting multiple builds. Lodash has a very customized build system to allow you to select only the parts of lodash you actually want to use.

@zackferraro
Copy link

Ah, this is a much better place. Thanks @montlebalm.

If F2 only wants to support one loader, then it ought to be AMD.

I agree.

If F2 wants to support multiple loaders, then do it the lodash way by supporting multiple builds.

And if my vendor/client wants to use a script loader that isn't in one of those builds then they're out of luck? Rather, if we allow the script loading to be a marginally restricted platform that anyone can use, then F2 just needs to supply those marginal restrictions (e.g. the scripts object in the manifest must be Array-like and the loader must support array elements that are URL strings) and that would allow anyone to use their own loader.

@montlebalm
Copy link
Member Author

jQuery is now completely AMD internally.

I believe jQuery is built with AMD to ease the creation of custom builds. The module wrappers are actually parsed out of the completed file.

https://github.com/jquery/jquery/blob/master/build/tasks/build.js#L44-L91

If F2 wants to support multiple loaders, then do it the lodash way by supporting multiple builds.

The difference between us and lodash is that we're expecting F2 to be a dependency shared by multiple 3rd parties. I worry that additional builds will undermine consistency in such a way that apps can't depend on define being on the page.

If we want to support loaders, I would suggest adding the ability to live in an AMD, CommonJS, or global context. I believe there exists boilerplate code to intelligently handle all three.

My one argument against loaders is that it's relatively easy to "shim" global libraries into an AMD context. By leaving F2 agnostic, we might play nicer with everything.

@montlebalm
Copy link
Member Author

I think the trickiest part of loader support is going to be app definitions. AMD would like each app to be wrapped in define. Non-AMD apps would likely prefer F2.define or similar. It seems as though there are a few options:

  1. Force all apps to use F2.define, which would mimic the AMD define method. The obvious downside is that it would require F2 to be a global, which is counter to a philosophy of AMD.
  2. All apps are wrapped in define. If AMD is not present, F2 would provide a basic implementation.
  3. Require every app to have some conditional boilerplate to define themselves as either AMD or F2.

Option 2 strikes me as the most reasonable approach. It feels a bit strange to add an additional global, but we could run it past some app developers and see what they think.

@ilinkuo
Copy link

ilinkuo commented Jan 25, 2014

And if my vendor/client wants to use a script loader that isn't in one of those builds then they're out of luck?

I suspect that you're arguing the case of a ghost here, @zackferraro, as jQuery and Underscore, with a user base hundreds of thousands times larger than F2, hasn't found a need to try to accommodate a loader other than AMD. Even lodash is adding only support for ES6 (experimental, forward-looking) and NodeJS/CommonJS.

If your client is wanting support for ES6, then they're really far out there, and they should work on their own separate branch of F2 and issue a pull request back to the main branch when they've worked out all the issues, because it's not going to be as simple as providing a little custom loader hook.

If you client is wanting support for NodeJS, then they'll need a significant rewrite of F2 because the existence of the DOM is one of the fundamental assumptions of F2, and again, a custom loader hook isn't going to cut it.

Even with AMD, a custom loader hook approach just isn't going to work, in my opinion.

@ilinkuo
Copy link

ilinkuo commented Jan 25, 2014

@montlebalm, I'll first respond to a few of your points and then outline what I think AMD ought to mean for F2.

The module wrappers are actually parsed out of the completed file.

I didn't know that, and that's actually a good thing to know for later ....

If we want to support loaders, I would suggest adding the ability to live in an AMD, CommonJS, or global context. I believe there exists boilerplate code to intelligently handle all three.

The umd specification handles this, but I haven't tried it myself yet. F2 can certainly be packaged as a umd module and it may load fine, but it's going to throw errors all over the place once your try to run it because it relies on the DOM which doesn't exist. And even if all the errors are removed, there's still the question:

In a world without a DOM, what is the meaning of life for F2 whose sole purpose is to load F2 Apps into a DOM container?

@ilinkuo
Copy link

ilinkuo commented Jan 25, 2014

@montlebalm, your vision of F2-AMD integration seems rather different from mine, and it's difficult for me to wrap my head around it at this moment.

These are my thoughts on what AMD-ifying F2 means.

  1. F2 ought to be rewritten internally to use AMD for its modularity.
  2. Once internally AMD-ified, F2 can choose either of the three alternatives:
    • Remain externally agnostic -- In this case, F2 can pass along the benefits of AMD to the user by offering customized agnostic builds like jQuery does (and the AMD can be stripped out in the built file just like jQuery).
    • Change the API to require users to adopt AMD -- In this case, Dojo is a good case study. Dojo used to be a singleton just like F2, but after AMD-ification, the singleton was broken up so that every piece of Dojo functionality became an individual AMD module. The F2 singleton would be similarly divided up into "F2/Constants", "F2/Events", etc.
    • Provide both AMD and agnostic customized builds -- This is the best of both worlds but more work. In addition, F2 needs to provide guidance on how plugins should be written so that they can operate in both AMD and non-AMD builds.
  3. Replace F2's custom App loader with an AMD loader -- The current loader loads duplicate resources, suffers from race conditions, and has other issues which a mature loader like AMD would have already solved. This would not be a simple backwards-compatible drop-in replacement because the script contexts are different when loaded by AMD, but it is worth it in my opinion.

Note that 3 doesn't actually require 1 or 2 as a prerequisite, so this can all happen in separate phases/releases.

A lot of your previous comments focus on the uses of F2.define, but I don't think that's something that's needed by my outline.

@brianbaker
Copy link
Member

I think its important to remember a few things that have been left out of the comments above. First off, to get an app on the page the following occurs:

  1. JSON/JSONP request for the AppManifest
  2. <link> tags added to the page for AppManifest.styles
  3. <script> tags added to the page for AppManifest.scripts
  4. <script> tags added to the page for AppManifest.inlineScripts
  5. HTML added to the page for each AppManifest.apps
  6. AppClass is instantiated for each AppManifest.apps

Replacing F2's custom App loader with an AMD loader really only applies to # 3 above, though a jsFiddle test would be good to try it out as well for # 1. The race conditions mentioned apply for # 1 and not # 3 due to JSONP and a static callback. # 2 isn't handled by AMD and would still need to be handled via a plugin or style loader. # 4 would likely be discouraged or taken out of the spec entirely.

6 is where a lot of @montlebalm's comments are focused because that is the largest area of change for app developers and in my view the largest change when deciding to switch to AMD. There are thousands of F2 apps in existence today and still only a handful of containers. So when we suggest to provide both AMD and agnostic customized builds, that is really only applying to how a container developer has chosen to architect their site. An app developer would still follow the spec and need to write their scripts accordingly (again, this is the crux of a lot of @montlebalm's comments surrounding define and F2.define).

@ilinkuo
Copy link

ilinkuo commented Jan 29, 2014

@brianbaker
Your analysis of AMD's effect on F2's App loading lifecycle is correct, but there's a potential for heavier involvement in the lifecycle.

2, css loading, is already available today as a one of several css plugins. I haven't used any of them so I don't know if they're implemented well.

To go further with AMD, F2 would have to start the implementation of a static manifest. While there are other benefits to the use of a static manifest, the benefit of concern to this thread is that a static manifest is cacheable, and it is these kinds of assets that are well-handled by AMD. To accommodate a static manifest in the F2 App loading lifecycle above, the existing App Manifest html should contain only placeholders (such as a loading progress display) and an extra step would have to be added to fetch the dynamic parts of the App Manifest

1.5 fetch dynamic html and data

This dynamic fetch step can be executed anywhere between # 1 and # 5, and could be executed in parallel with the steps after 1.

In particular, this static-ification of the App Manifest would be a natural consequence of moving to package.json, as @montlebalm noted.

With the dynamic fetch factored out, all the assets in steps # 1 through # 4 (excluding the dynamic html) can be loaded by AMD, the AppManifest/package.json is cached for future instances, both css and js resources are not loaded in a duplicate fashion and can be loaded in parallel, and there would be no need for F2 to maintain its own loader even for the AppManifest.

The following is an outline of how this might work

require([/* ...*/], function(){
  var manifestUrls = [ /* ... */];
  manifestUrls.forEach( function(url){
    require(["json!" + url], function(manifest){
      // For example, jsonp plugin at https://gist.github.com/millermedeiros/1255010, 
          // but I haven't tried this
      var dependencies = [];
      dependencies.concat(manifest.scripts);
      dependencies.concat(manifest.styles.map(function(cssUrl){
        return "css!" + cssUrl;
      });
      require(dependencies, function(/* named list of deps */){
        manifest.inline.forEach( function(inlineScript){
          eval(inlineScript);
        }
        var dynamicHtml = jQuery.get(manifest.htmlUrl);
        insertIntoDom(dynamicHtml);
      }
    }
  };
});

Of course, this won't run as is because scripts need to be wrapped in an AMD define or require as well as conform to some other AMDisms, but this expresses my thoughts.

I-Lin Kuo

@brianbaker
Copy link
Member

Today, many manifests return combined and minified scripts. How would that work in the AMD world? I've never had luck combining many AMD modules into a single file that I could load into the page after page load. It seems like apps could no longer combine scripts together and would instead need to list them all individually?

@ilinkuo
Copy link

ilinkuo commented Feb 9, 2014

@brianbaker I'm not sure I'm answering your question, so I'll give a later answer as well.

There's a need to get into some AMD details to answer this question. In AMD, each module is a single file

define([ /* dependencies */, function(){ //...
});

This is great for development but not recommended for production as there are too many http requests. For production, it is recommended that all the modules needed are concatenated into a single file, called a layer. The AMD implementation will provide a tool for this -- dojo build for dojo, r.js for RequireJS -- which does this. The tool does a little bit more than concatenation and minification as it traverses the AMD dependency tree and then slightly modifies the module definition

define([ "tda/scanner", /* dependencies */, function(){ //...
});

to add the moduleID, but that's about all it does.

In addition, there is a config mechanism in AMD implementations that allow you to tell the loader to load the layer first and only perform actions after the layer is loaded. If this is not done properly, the code will simply attempt to download the modules it needs as single files rather than as part of the zip. This is a point of fragility in using the AMD layering, because if you forget some dependency in the layer, you'll see extra http requests for missing modules. Thus, I recommend that you publish the individual modules along with the layer so the site still works if there are modules missing from the layer.

This mechanism is good enough for most sites.

However, with F2, there are going to be some complications. An F2 App would, I think, prefer to zip everything up rather than load individual modules. For an F2 App, zipping everything into a single layer works if you only have only one instance of that app on the page. If you have multiple instances, or if you have different F2 Apps sharing some common modules, then your layers will overlap and you will get "multiple define" errors. These errors won't break the page, but will show up annoyingly in the console. The problem is that AMD prevents duplicate module loading but doesn't have a mechanism for preventing duplicate layers loading.

It's a problem that actually exists today in F2, albeit in a different form. In the MOD F2 Apps I've looked at, the minified statements seem to run bare, as in

mod.rss = function(){ /* ... */}

There's usually no guarding by testing for existence, so when multiple instances are run, the above statement is executed for each instance. Unlike AMD which calls attention to this by throwing a multiple define error, the current F2 loader just silently runs the code multiple times, possibly over-writing singletons if not careful.

In current F2, the simplest solution is to put a guard

if (!mod.rss){
  mod.rss = //...
}

This guarding doesn't quite work for an AMD'ed F2, because if you put a guard inside a define, you're still going to get the multiple define error. If you put a guard outside of each define, then you're kind of duplicating AMD's loading. The best place to guard is around the entire layer.

if (!layers.mod.rss){
  // layer definitions
  define("mod/utils", /* ... */);
  define("mode/date", /* ... */);
 // ...
}

This works for removing the multiple define errors for multiple instances of a single app, but it doesn't solve the problems arising when different apps use the same library (that's a question I'll leave for another day unless someone is interested in it now).

@ilinkuo
Copy link

ilinkuo commented Feb 9, 2014

@brianbaker If instead, your question

a single file that I could load into the page after page load.

is about generic AMD loading, and not about F2, then I'll try to answer that in this comment.

The AMD loading mechanism with the config is designed to load the layer first and then run the requires. Because a layer is just a bunch of defines with the module ids filled in, if you wrap the minified layer itself with a define, as in

define("my/layer/bbaker", [], function(){
// put the r.js produced minified layer here
  // define ...
}

Then you can treat the layer itself as a module and load it in the normal way.

The problem is that when you do this, you need to make sure that none of the modules in the layer are in the dependency chain of what's already loaded, otherwise you'll get some "multiple define" errors.

@ilinkuo
Copy link

ilinkuo commented Feb 10, 2014

The full solution for layering with AMD'ed F2 is the following, I think:

  1. An App provider that has only a single app can do the simple thing and use a single layer.
  2. App providers that have multiple apps need to factor out common parts into a layer and use a multiple layer strategy.
  3. F2 has some layer loading strategies to choose from (probably not an exhaustive list, details missing):
    • Have the manifest specify layers separately from scripts and load those separately before the scripts. The loading should be guarded so as to not load duplicates. The framework can put in the guarding.
    • Require layers themselves be wrapped in a define as an AMD module, but not handle them separately as layers.

@brianbaker
Copy link
Member

I tried to put together a jsfiddle with an F2 layer and a layer for the app. Its kind of involved and was a little cumbersome to put together in jsfiddle/github - next time maybe I'll try plunker to see if its any easier for this type of example.

http://jsfiddle.net/rekabnairb/gW8B5/

@ilinkuo
Copy link

ilinkuo commented Feb 21, 2014

@brianbaker That's great! Thanks for putting together a fiddle.

I didn't understand what you mean by "kind of involved" -- The fiddle looked pretty simple to me and https://github.com/brianbaker/brianbaker.github.io/blob/master/js/f2-amd.js looked like a stripped-down version of F2. Did you mean the nested requires in f2-amd-js?

@brianbaker
Copy link
Member

@ilinkuo "involved" as in I wasn't able to have it self contained within the fiddle but rather it had to include several external resources.

The nested requires are where I had some issues getting the app class and its dependencies loaded correctly. For example, if I tried to load the scripts from the Manifest directly by url (ex. define(manifest.scripts, ...)), the modules within the layer would not be loaded and require would try to pull them from the wrong url when they were requested. Instead, using require.config() to add additional paths allowed the layer to load properly and then the nested require within there would load up and instantiate the app class itself.

The other issue I had was with jQuery - it wants to define itself as the module 'jquery'. I massaged the minified source to rename it to 'jquery-1.11.0'. I'm sure there's probably an easier way to do that.

@brianbaker
Copy link
Member

Some thoughts from the jsfiddle exercise:

  • Within the app's layer, you cannot reference other urls such as define('jquery-1.10.0', ['//jquery/cdn'], ... ); (or at least I couldn't get it working) because they just won't get loaded properly. The module has to be loaded with its full source in there. This isn't really an issue though as various optimizers can do that easily.
  • It would be good to demonstrate an app pulling in a separate "common" layer, as I've heard quite a few developers wondering how to properly include "common" scripts in addition to their app class.
  • Do we allow app developers to shim in other non-amd libraries? If so, what does that end up looking like in the manifest?

@brianbaker
Copy link
Member

If you're so inclined or want to play around with Plunker, here is the example over there: http://plnkr.co/edit/rincAHMSB09OcW8kduVy

@ilinkuo
Copy link

ilinkuo commented Feb 24, 2014

The nested requires are where I had some issues getting the app class and its dependencies loaded correctly. For example, if I tried to load the scripts from the Manifest directly by url (ex. define(manifest.scripts, ...)), the modules within the layer would not be loaded and require would try to pull them from the wrong url when they were requested. Instead, using require.config() to add additional paths allowed the layer to load properly and then the nested require within there would load up and instantiate the app class itself.

I think the value of "//brianbaker.github.io/js/apps/helloworld/io_github_brianbaker_helloworld.layer" won't be loaded properly. I think (not completely sure), the two "//" to start might be an AMD no-no in the dependencies list (but OK in the paths). The current value looks like an AMD Module Id and it might get confused. To get AMD to treat it as an actual URL, you might either have to use the full url, or to add a ".js" to "...helloworld.layer". AMD will treat a value beginning with "http" or ending in ".js" as a url instead of a module ID. These alternatives might work, but I think the way you did it is the canonical way.

I'm going to play around tonight to see if jquery can be loaded separately as per instructions here.

Do we allow app developers to shim in other non-amd libraries? If so, what does that end up looking like in the manifest.

I think there's plenty of pre-processing choices to make, depending on how you want to set the policy. In my example outline, there's pre-processing which is done to add "css!" prefix to all the manifest.style elements. You are free to do any kind of pre-processing or filter to allow/disallow urls for manifest.scripts as well.

@ilinkuo
Copy link

ilinkuo commented Mar 12, 2014

Sorry for the delay. I've forked the github repo and the jsfiddle to create an example of two hello_worlds and two hello_TDAs loading. These are essentially the same as @brianbaker 's original hello world, but I've just refactored out the jquery into its own separate layer which is shared by all the hello_XXX's. See http://jsfiddle.net/F6c9Q/5/

@brianbaker brianbaker removed the v2 label Jul 20, 2021
@brianbaker brianbaker removed this from the 2.0 milestone Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants