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

Prevent ModuleConcatenationPlugin from inlining view models. #154

Closed
shabalin opened this issue Sep 26, 2018 · 4 comments
Closed

Prevent ModuleConcatenationPlugin from inlining view models. #154

shabalin opened this issue Sep 26, 2018 · 4 comments

Comments

@shabalin
Copy link

I'm submitting a bug report

We were very excited with the news that there is no more need to PLATFORM.moduleName dialog view models, so we decided to give it a try.

https://aurelia.io/blog/2018/09/06/aurelia-release-notes-early-september-2018

Usage of Origin.get was removed; we now depend on the built-in support for class references from the CompositionEngine. What does this mean? Usage of class references for .viewModel should no longer cause issues with Webpack when PLATFORM.moduleName() is not used.

We removed all dialog VMs PLATFORM.moduleName() statements in our codebase, and were very happy to see dialogs working when we run app in webpack development mode. Unfortunately, the behavior is different in production mode. We prepared a repo based on Aurelia CLI app template that reproduces and issue.

  • Library Version:
    2.0.0-rc.3

Please tell us about your environment:

  • Operating System:
    OSX 10.13.6

  • Node Version:
    v10.6.0

  • NPM Version:
    6.4.0

  • JSPM OR Webpack AND Version
    webpack 4.16.4

  • Browser:
    Chrome 68

  • Language:
    TypeScript 2.9

Current behavior:
If i build a bundle with mode: "production" I am getting the following error on dialog opening.

aurelia-templating.js:792 Uncaught (in promise) Error: Cannot determine default view strategy for object.
    at e.getViewStrategy (aurelia-templating.js:792)
    at aurelia-templating.js:4802

in development mode dialog does open with no errors

I was able to reproduce the issue for the clean Aurelia App template (created using latest AURELIA CLI)
https://github.com/shabalin/aurelia-dialog-issue-361


Initially we reported this problem to the Dialog repo, please read
aurelia/dialog#361 (comment)

@jods4
Copy link
Contributor

jods4 commented Oct 11, 2018

@bigopon Can you have a look at this one?
You created the new apis that don't rely on module and export names but it seems something still won't work if you hoist a VM as a child of another module.

@bigopon
Copy link
Member

bigopon commented Oct 11, 2018

@shabalin I think its behavior is a bit unfortunately confusing. The mandatory usage of PLATFORM.moduleName was dropped for aurelia-dialog was mainly to leverage new features of aurelia-templating, which you can use static reference to a plain class, instead of a string as module name, to be the dialog view model. On the surface, there seems to be nothing has changed, but there are differences between v1 and v2.

  • For v1, no matter what you use, string, or class, aurelia-dialog always tries to ask loader (in your case, it's aurelia-webpack-loader) to find the Origin or path, that represents the module of view model, and uses that to find the view url. This is why PLATFORM.moduleName is required for v1, because without it, aurelia-webpack-loader fails to find the Origin of the view model (note it's only specific to webpack), thus will fail to find url for the view (read convention view strategy).

  • For v2, it stops trying to figure out the Origin of the view model, if a class is given, it will simply uses new feature of templating to find view strategy. It works in dev build because modules are preserved as is without hoisting, so the loader has the information to know where the view model class comes from, but not so lucky when it's hoisted.

So dropping PLATFORM.moduleName in the doc means drop the enforced magic, it does not really mean more magic.

@bigopon
Copy link
Member

bigopon commented Oct 11, 2018

What we can do, to make it less surprising, is to add a warning inside aurelia-dialog to warn about this behavior specifically for webpack loader. Thoughts ? @StrahilKazlachev @jods4

@jods4
Copy link
Contributor

jods4 commented Oct 14, 2018

I am going to close this issue, because I don't think there's anything to be done about it on the webpack plugin side.

Preventing module concatenation is exactly what this plugin is about, through various means such as PLATFORM.moduleName or GlobDependenciesPlugin. So one solution is to use them.

If you want to go the pure JS route, we're adding APIs for that and the solution to your problem is to declare your view explicitly. Here's @StrahilKazlachev answer from the aurelia-dialog thread:

For this to work you will either need to pass the .view(moduleId or ViewStrategy) or use @view()/static $view on the view model class.

I agree the guidance could be better. At least the error message should lead you to the solution above (adding $view) rather than leaving you wondering what's wrong. This is best discussed in aurelia/dialog#361

@jods4 jods4 closed this as completed Oct 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants