Dynamic Aurelia dependencies about pal HOT 34 CLOSED

Not sure how to solve the basicConfiguration() vs standardConfiguration(). I've been thinking about this too some time ago, but dropped the matter as most users use standardConfiguration anyway. But it would be good to have this configurable.

From the discussion with @EisenbergEffect, I won't add moduleNames for basic configurations. Rather, the build tooling will propose equivalent default configs that you can pick from.
The default will include everything so that it "just works", and then people can remove pieces that they don't use if they want a minimal size.

A "custom" configuration happens in user code, which means moduleName.

from pal.

Checking protractor. Doesn't matter if it's published yet, just want to make sure we have everything covered.

from pal.

Just spent some time with user mike-suggit who got his tests with Karma + Jasmine + aurelia-testing + Protractor working.

Was a bit rough because he had quite a few bad configs from existing build. But once everything was referencing the correct packages, the config properly updated, moduleName added where required, it all worked.

So although I did not look at it, I'm going to check that aurelia-testing checkbox.

from pal.

The main location is going to be the bootstrapper, I think.

from pal.

Regarding your question, I think we took a shortcut in that we put most of the aurelia.build.resources in the bootstrapper package, instead of the respective packages. There aren't many dynamic requires throughout Aurelia, so this should be a pretty simple task.

from pal.

I managed to get my demos running without using aurelia.build.resources. I did not modify aurelia modules yet, instead I used a plugin of mine to introduce the dependencies I wanted in my build, like so:

new ModuleDependenciesPlugin({
      "aurelia-bootstrapper": [ 
        // "aurelia-loader-webpack",    // detected by webpack itself
        "aurelia-pal-browser", 
        "aurelia-framework", 
      ],
      "aurelia-framework": [
        "aurelia-history-browser",
        "aurelia-logging-console", 
        "aurelia-templating-binding",
        "aurelia-templating-resources",
        "aurelia-templating-router",
        "aurelia-event-aggregator",
      ],
      "aurelia-templating-resources": [
        "./compose",
        "./if",
        "./with",
        // [...] some more skipped for brievity
      ],
      "aurelia-templating-router": [
        "./router-view",
        "./route-href",
      ],
    })

Most of it will be trivial enough, e.g. templating-resources or templating-router.

But I think we need to have chat about bootstrapper and framework. The culprit is that those dynamic loads may not occur, depending on your configuration or your environment. And we most certainly don't want to embed those dependencies in the build if they are not used. For instance, I only listed one of the three PALs.

from pal.

I don't think there's any use case for having more than one PAL bundled. In some cases you might want to have a separate bundle for pal-worker, but it would still only contain that one PAL.

from pal.

@niieani Yes, that's what I'm saying. But the problem is that choosing which one is included must be done at build time and can't be declared inside aurelia-bootstrapper.

My thinking for aurelia-bootstrapper is:

• the convoluted syntax used to import aurelia-loader-webpack is automatically picked up -> no change;
• the PAL must be decided by the build tool and should not be wrapped with moduleName;
• aurelia-framework is normal, should be wrapped with moduleName.

For aurelia-framework the problem is that all six modules are standard but optional configs. If I wrap them with moduleName everything will work but someone using basicConfiguration() instead of standardConfiguration will not want the router in the bundle.

Thoughts?

from pal.

What is bootstrapper-webpack?
Despite its name I don't use it in my webpack builds...

from pal.

@jods4 bootstrapper-webpack was the webpack-specific bootstrapper code, which we will be deprecating soon, because we managed to get together a universal aurelia-bootstrapper with build-system detection for all our use cases (hence the "convoluted syntax" - its the only one that does not trip up SystemJS, RequireJS and works with Webpack when needed by not adding extraneous dependencies).

Your thinking is correct.

Not sure how to solve the basicConfiguration() vs standardConfiguration(). I've been thinking about this too some time ago, but dropped the matter as most users use standardConfiguration anyway. But it would be good to have this configurable.

from pal.

Sounds good.

from pal.

OK, I have 4 PR open referencing this request.
As you can see in the checklist this covers all "core" aurelia modules and is enough to be able to build an app that doesn't use optional modules without any extra manual dependencies.

(well, considering that the default extra dependencies are added by the build tool, as described above)

from pal.

Reviewed all. Great comment work, looking good to me!

from pal.

Thanks @niieani, can someone merge them? They're relatively trivial.

@EisenbergEffect I've looked at almost all modules (see the checklist). Would be nice to get help for the remaining ones.
I am pretty sure the following modules need changes:

• dialog
• validation
• html-import-template-loader (it fetches webcomponentsjs/HTMLImport.min through the loader).

For the rest I don't know.

from pal.

@EisenbergEffect I see that you are merging all my PR above and about to publish the new webpack tools.
It would be awsome if some Aurelia members could have a quick look and test with the items in this list that are not ticked yet.

Some might require no change at all, some might not work out of the box.

from pal.

There's a problem with dialog (and possibly with other external plugins) that is a bit harder to solve. See this part of the code.

This is currently causing errors such as aurelia/dialog#203.

In short, everything works as far as you provide a string path to the ViewModel and let Aurelia load it, rather than passing an actual class. The problem is that the Origin is set by Aurelia whenever a module is being loaded by the loader-webpack, unless it was in Webpack cache before eachModule is executed. However if you import a class directly, Aurelia will have no knowledge of the file that it came from. For reference, the Origin is the reverse-map of exports to moduleIds, used to "guess" things like the path to the related View using a convention, so this problem is not specific to Dialog, but to all the times an Origin is expected but the underlying module has not yet been touched by the Loader.

This is also the reason my original v2 Webpack solution was naming all the Webpack modules, as opposed to the @jods4 re-written solution which only names those specifically referenced by PLATFORM.moduleName. This is what used to happen in v1 and leads to problems like aurelia/dialog#241.

One semi-solution that comes to my mind would be to hook Webpack's chunking mechanism to run the Origin setter right after loading a given chunk. This would only work if all the modules had actual names, since it actually concatenates moduleId + .html, hence the above problem of Cannot find module './76.html'. The big downside of this is that all the modules in the whole chunk would get evaluated immediately, as if they're all imported, just so we can set their Origin.

Another two solutions would be breaking changes, but overall reduce complexity in some ways:

1. instruct users to user PLATFORM.moduleName even for ViewModels imported with import {} syntax if they plan on using them this way, while at the same time add logic to PLATFORM.moduleName which runs the ensureOriginOnExports logic on all the referenced modules. Not great.
2. disallow passing imported ViewModel classes directly, ask users to always specify paths as strings so they go through the loader. Simplest, but breaking users and plugins.

Any better ideas?

from pal.

@niieani So passing VM instance is also ok since it was already loaded?

from pal.

@StrahilKazlachev if it was loaded by Aurelia, and not via an import, it is ok currently, and will be okay with v2 of Webpack stack.

from pal.

@niieani I think we need to chat live about this.

To find the view, using @useView should be a work-around, right?

But the whole Origin thing is surely going to be broken for all classes that are not loaded through aurelia-loader.

from pal.

Yeah, explicitly decorating the ViewModel with @useView does work; not depending on the conventionally named views solves this. But currently the experience is sub-par, since it's that "sometimes" it works automatically, and sometimes it does not, and the user has no idea why.

Still, one of the nice things about Aurelia is that you don't need to use useView explicitly for every ViewModel.

I'm open to chatting about this on Gitter.

Thoughts about this one, @EisenbergEffect? This is a problem especially with code loaded on demand, since even if we consider re-running PAL.everyModule with each new file, we would also evaluate all the modules inside it, as it loads. Not sure how is this solved in RJS or SystemJS (do we even support loading modules on demand in the latter?), unless its not, and we just missed a bug?

This is also one of the blockers that keeps us from implementing logic that replaces moduleNames with random names for security, since it simply tries to append .html to the end.

from pal.

The situation is even worse with SystemJS since there's no logic at all there.
Reference logic for eachModule under RJS and SystemJS here.

Having just looked at the implementation of Origin, it seems that when origin is undefined, it is to be retrieved and set on demand, as the get call occurs. Having said that, there might be a less-then-perfect way to do this with Webpack, since we know for sure a module should be in the Webpack cache once any imports have occured, we could just iterate over the whole cache and all of the keys of each element until we have found a matching function/class. The performance of this operation would be pretty bad though, however, if it's any consolation, this operation would only have to happen once (per each undefined Origin). With @jods4 approach of only naming modules which have been explicitly added via PLATFORM.moduleName, the user would still need to re-declare the imported file with a call to that function. If we renamed all modules as in my approach, the imports should work automatically.

Another thing is that our NodeJS loader will behave the same way as Webpack currently does (i.e. errors related to this will happen), since as far as I know there is no way to find out the file path from an already loaded reference to a function/class, and since there is no bundling involved here, running eachModule equivalent to set Origin for all js files under the root directory is probably out of the question (startup performance would be abysmal). Perhaps we could use some hacky node APIs to list all cached modules, but these APIs are explicitly defined as unsupported/deprecated in the NodeJS documentation, so even if we solve the Webpack problem somehow, I'm not sure how we'd do this for Node, and we'll need to support it if we want server-side-loading, going forward.

from pal.

@niieani
Trying to make some progress here:

• I think you made protractor work with the new build, can I check it?
• Now that I added support to preserve the name of modules that are vm of views by convention, is aurelia-dialog ok?

@EisenbergEffect
Is html-import-template-loader still a thing? I vaguely remember using this when I got started with Aurelia a long while ago but never using it since then... What does it do? Should I remove it from the list?

from pal.

@jods4 protractor is good since the updates that made it execute scripts synchronously. Don't remember if we published that to NPM yet or not.

Haven't retested aurelia-dialog yet.

from pal.

@jods4 - can you check validation off please- I've made the platform changes but I don't have privileges to update issues in this repo. Thanks!

from pal.

Thanks, I checked validation off the list.

from pal.

@jods4 could you share how you and mike-suggit configured karma to work with aurelia-testing?

from pal.

I don't remember, better ask @mike-suggitt directly!

from pal.

It doesn't load aurelia-ux due to the ${$design} interpolation inside .css files.

from pal.

@khuongduybui I don't think this thread is the right place, you should file an issue in aurelia-ux repo.

This meta-issue is to keep track of where PLATFORM.moduleName were added in core aurelia libraries.

What you have sounds like a loader issue and should be tracked separately.

from pal.

Ah okay, thanks. I'll file the issue in ux repo instead.

from pal.

@jods4 the html-import-template-loader module doesn't seem to need it. I think it can be ticked.

from pal.

@bigopon Thanks for that! I checked it.

from pal.

Et alors?
(-:

something about sprinkling moduleName all over the place
just a ping. seems almost baked.

from pal.

Closing since I think all libs that require this have been instrumented.

from pal.