aurelia / loader-webpack

An implementation of Aurelia's loader interface to enable webpack.
MIT License
26 stars 10 forks source link

My feedback of module loading and webpack support in Aurelia. #14

Closed d-ph closed 7 years ago

d-ph commented 7 years ago

I'm submitting a feature request

Hello,

I'd just like to share with you a few opinions of mine about Webpack support in Aurelia.

I think things would be a lot easier for everyone, if all dependencies between files in Aurelia were discoverable statically from just looking at the source files. This is what makes Webpack shine and work without causing headaches and without need for additional plugins.

This is somewhat completely the other way around in Aurelia, which makes heavy use of the Loader class, which offers loading of js/html files dynamically. Because of that, in order to run Aurelia, there is a need of adding the dedicated aurelia webpack plugin, that I personally am not very fond of using.

So, my 2 cents are: if Aurelia team really, really wants to offer support for 2 different styles of loading dependencies (require.js/system.js' async style vs webpack's compilation style), then it'd be great to (a) offer a way to use the framework in a more webpack-friendly way and (b) make the core framework code load dependencies statically instead of dynamically via the Loader class.

E.g. instead of:

aurelia.use.plugin('aurelia-dialog');

offer a way to install this plugin statically:

import { AureliaDialogPlugin } from 'aurelia-dialog';

aurelia.use.plugin(AureliaDialogPlugin);

Instead of loading aurelia-framework like this in the webpack bootstrapper:

loader.loadModule('aurelia-framework')

use es6 import:

import { Aurelia } from 'aurelia-framework';

There are features that require dynamic module loading. I suppose this is how Aurelia's able to load a matching .html view file for a .js viewModel file without mentioning the .html in any way. Well, I'm quite ok with importing the html file via var viewHtml = import 'my-component.html'; and then using the @useView decorator to inform Aurelia explicitly what html to use.

Instead of using the <require> tag in html file like this:

<require from='./nav-bar'></require>

I would be happy to use a decorator on relevant components like this:

import { NavBarComponent } from 'nav-bar'

@useComponent(NavBarComponent, /* as */ 'nav-bar')
export class App {
  configureRouter(config, router) {}
}

The only occasion, when modules should be able to be loaded asynchronously and when the framework should step forward with an opinion of how to do this, is when there is a requirement to load a Router's route on demand. Please take a look at one of the official webpack examples of how this could be done: link. This is very similar to what loader-webpack does (the requireing of "aurelia-loader-context/" prefix idea), but it uses one of the official loader plugins: bundle-loader. There is also an angular-webpack project around on github, where guys use es6-promise-loader instead.

Where I'm trying to get at is: this would be really helpful, if Aurelia (and all official ecosystem) was not using the loader in the framework code and let the actual developer decide, whether they want to have some more syntactic sugar and convenience (like the <require> html tag), that is available only, when require.js-style loader is in use, or whether they want to use webpack-style loader and sacrifice those additional features.

I could reason more about it, but this is already getting long and nobody likes reading walls of text.

So there it is -- my feedback and opinion of the state of affairs. I might be wrong in many places -- I've only started with Webpack a few weeks ago. Also, I know, that Aurelia devs, and previously Durandal devs, are very accustomed to having the ability to load stuff dynamically (previously with require.js and now with system.js). So having stated that, I'm perfectly fine with Aurelia just carrying on with its course -- no hard feelings. I just thought, that hearing another dev's opinion and point of view might be helpful for Aurelia's devs, while developing this fantastic framework.

Thanks for spending your time to read this.

niieani commented 7 years ago

First of all thanks for taking the time to think about this. It's really important for us to hear and react to external feedback. Kudos!

Now to the matter at hand: I've actually already thought about this in the past, and as a maintainer of the Webpack components, I've asked the very same question to the designers of Aurelia and Aurelia's router: why pass path-strings instead of actual Objects - for routes, for required elements, plugins, etc.? It turns out to be a very well thought out strategy and a conscious design decision that allows large projects to have their modules truly independent.

@EisenbergEffect explained this well in the past, but I'll give it a go myself. Probably should go into the docs where others in the future can be pointed to.

If we'd use Object passing-everywhere strategy problems arise:

app.js:

import { NavBarComponent } from 'nav-bar'

@useComponent(NavBarComponent, /* as */ 'nav-bar')
export class App {
  configureRouter(config, router) {}
}

The above means the 'nav-bar' file absolutely has to be statically included in the bundle which contains app.js. There's no way around it - even if you use bundle-loader or es6-promise-loader, you'll only separate the nav-bar file out to its own bundle. This is not a problem in the case of elements which are always required (like the nav-bar in main), but starts to become one in a Router scenario. In order to just define routes (not load them!), you'd need to load all the bundles of all the descendent routes!

Imagine defining routes according to your idea, like this:

app.js:

import {Welcome} from './welcome';
import {Users} from './users';
import {ChildRouter} from './child-router';

/// ...
      { route: ['', 'welcome'], name: 'welcome',      module: Welcome,      nav: true, title: 'Welcome' },
      { route: 'users',         name: 'users',        module: Users,        nav: true, title: 'Github Users' },
      { route: 'child-router',  name: 'child-router', module: ChildRouter,  nav: true, title: 'Child Router' }
/// ...

Lazy-loading of all the routes: Welcome, Users, ChildRouter doesn't help you at all. You still need to load all of those modules in order for the Router to create the instance of the router! Angular2's router has bumped into exactly this problem, and that's why they had to completely redesign it recently, even though they're already in RC. This works for small projects, but doesn't scale. The current way - passing "moduleIds" instead of modules themselves, is the sane solution to this problem. If you have better ideas, feel free to share!

sane-app.js:

/// ...
      { route: ['', 'welcome'], name: 'welcome',      moduleId: './welcome',      nav: true, title: 'Welcome' },
      { route: 'users',         name: 'users',        moduleId: './users',        nav: true, title: 'Github Users' },
      { route: 'child-router',  name: 'child-router', moduleId: './child-router', nav: true, title: 'Child Router' }
/// ...

EDIT: One alternative that would be webpack-specific came to my mind - instead of embedding the module in the Router, embed a function that loads the module (which is possible with the bundle-loader. The problem is, even if we'd support this, you'd still need the webpack-plugin, because external plugins have to be developed in a way that works across toolings, i.e. they need to work both for Webpack and other module-loaders.

d-ph commented 7 years ago

Hi niieani,

Thanks for such an extensive reply. I really appreciate that.

Regarding the router scenario: I completely agree, that importing every module to configure the router is an insane idea (because devs would essentially end up with 1 big javascript file, that contains all the source code (and by the way, I am surprised, that there were people that thought this would work out)). On the other hand, solely using async loading in the router is also a bit less than ideal. My opinion here is: the framework should allow the end developer to choose for himself, which loading style they want to use for their routes. E.g. I would like to load "master" routes of a SPA as separate js files, but also I'd like all child routes to be distributed in the same js file (webpack terminology: bundle) as the master route they are defined for. In the code it could look like this:

interface RouteConfig {
    route: string|string[];

    /*
     * Mutually exclusive properties. When the `module` is defined, then it's easy,
     * because the symbol is already loaded. When the `moduleAsyncLoadingFn` is defined, then the Router
     * will run that function to obtain the module asynchronously (and cache it). When both are
     * defined, then `module` takes precedence and a warning is emitted. When none is
     * defined, then that's a fatal error.
     */
    module?: RoutableComponentInterface;
    moduleAsyncLoadingFn?: () => Promise<RoutableComponentInterface>; // this typing is too strong, but I'm just trying to make a point here
}

/// ...
      { route: ['', 'welcome'], name: 'welcome', moduleAsyncLoadingFn: require('bundle!admin/welcome'), nav: true, title: 'Welcome' },
      { route: 'users', name: 'users', moduleAsyncLoadingFn: require('bundle!admin/users'), nav: true, title: 'MyApp users' }

      // alternatively, for a different loader
      // { route: 'users', name: 'users', moduleAsyncLoadingFn: () => { return System.import('admin/users'); }, nav: true, title: 'MyApp users' }
/// ...

/// ...
    // in `admin/users`

    import { UserEdit } from 'admin/users/user-edit';
    import { UserFeedbackHistory } from 'admin/users/user-feedback-history';

    // (...)

      { route: '/:userId', name: 'user-edit', module: UserEdit, nav: true, title: 'User edit' },
      { route: '/:userId/feedback-history', name: 'user-feedback-history', module: UserFeedbackHistory, nav: true, title: 'User feedback history' }
/// ...

This way in production I would end up with these 3 files (but with uglier names):

  1. app.js
  2. admin/welcome.js
  3. admin/users.js (includes: admin/users/user-edit.js and admin/users/user-feedback-history.js)

Nota bene, this would also work with System.js or r.js optimizer, which now know, that those child routes need to be distributed with the admin/users.js


Regarding the Aurelia's plugins ecosystem and the need for an abstract idea of module loading. Webpack-friendly code is already async-loaders-friendly. And for those occasions, when plugins want to expose a functionality, that requires async loading -- the docs could advise: "Take a look at how we did our router and do the same, please". So for example, if aurelia-dialog wanted to expose a functionality, where the dialog's viewModel is loaded asynchronously or synchronously (it's down to the end developer to decide (and by the way, nothing pisses me off more, than when I see, that a dialog opens with a lag, because before it's shown, it needs to asynchronously fetch the code for it)), then they would follow the same idea as I presented in the interface RouteConfig {} above.


The way I see it, Aurelia's Loader service is not supposed to be abstracting the module loading. It's supposed to be abstracting async module loading. The common denominator of async and sync module loaders is... sync loading. In other words: async loaders can load code in async and sync (by sync I mean: these loaders can bundle files together for production, like r.js optimizer), but sync loaders can load code only in sync (well, they can load code in async, but it's considered an edge case rather than a common use case, and is implemented by means, that should be under the control of the end developer, not framework's plugin, that exposes its own configuration and its own opinion, that might not be inline with the end developer's).

If the end developer wants to be able to use framework's features, that require async loading (viewModel/view convention over configuration, html tags etc.), then they need to acknowledge the fact, that they need to use Aurelia's Loader, that requires an async loader implementation. The price they will pay for the convenience is that in production they won't know which source files will be actually used by the application, so they need to minify and deploy all of them. The framework itself should not use the Loader class, so the end developer is able to make a conscious decision for himself.

I realize, that changing something so fundamental, when the framework is already in Release Candidate, is a brave thing. This is exactly what Angular2 did with their Router (which is now in version 3) and there is probably some amount of resentment around the fact, that they change something so fundamental so often (I certainly hated that). But they seem to be on good track with their Router now (see their docs, where they "promise", that they know that there is a requirement for async Routes loading and that they will document how to do it soon). I guess progress is always painful.


I know that require.js, and its async module loading, is with us since forever now. Countless of successful projects has been written with this async loader (including some of my own), so everything I'm saying (and all people advocating the use of webpack) probably sounds like heresy. Also, configuring the Router with moduleId: './welcome' is so much brain-friendly than moduleAsyncLoadingFn: require('bundle!admin/welcome'). I guess it's natural, that more power requires steeper learning curve, otherwise all of us would be all-powerful from the get go. But that's beside the point. What I'm trying to say here is, that I'd understand if my feedback would just be taken into consideration without tangible effects. I.e. if Aurelia kept following its course. Javascript browser module loading is a complex matter. It's 2016 and we still have x competing standards for solving this problem. I'm ok with the fact, that Aurelia, in its pursue for great developer experience, decides to make opinions, that satisfy most of the devs. I regret the fact, that I'm not in the majority of the devs, but I'm not a fan of the minority impacting the majority either.


Thanks for reading this reply and my feedback as a whole. This is the last one this long in this thread -- I've said everything I wanted now.

niieani commented 7 years ago

I agree with you for the most part, although as you say, it's a bit too late to make such fundamental changes to the way we load modules. It's possible to implement an additional method of loading modules, but it's ingrained in so many of Aurelia's modules (and the ecosystem) that it would be a big endeavor. Feel free to fork and experiment - if you're successful we'll consider merging the support into Aurelia.

As for the moduleId: './welcome' vs moduleAsyncLoadingFn: require('bundle!admin/welcome') -- whichever way you go, you end up with a string, which is non-refactorable. When the in-browser module loading standards arrive, we can think how best to integrate those standards into Aurelia. Right now, I just don't see enough benefit of building-in support for a single module bundler (Webpack), since the problems we're currently faced with can be rectified with a simple plugin.

Awesome feedback, once again, @d-ph.

d-ph commented 7 years ago

Ok.

Thanks again for spending your time going through and understanding my point of view.