aurelia / dialog

A dialog plugin for Aurelia.
MIT License
106 stars 115 forks source link

feat(resources): register resources by class #354

Closed StrahilKazlachev closed 6 years ago

StrahilKazlachev commented 6 years ago

Register the optional resources by class and drop the usage of PLATFORM.moduleName. Since the resources are optional, when someone turns off their registration(using the plugin config API) it is also expected not to want them bundled. Currently they are being statically imported/reexported so I doubt excluding is easy. The following changes were made to simplify this scenario:

BREAKING CHANGE - default resources are no longer reexported, need to be explicitly included when bundling

So I just couldn't come up with a solution not using import(), so if its usage currently is unwanted I'll have to just keep the current approach with the static imports. That is if someone does not suggest something else.

I did only try the amd/es5 build with a CLI generated project(requirejs/babel) and it worked. Only had to set "resources": resources/*.js in the dialog config to get them bundled.

@bigopon @jods4 @EisenbergEffect

bigopon commented 6 years ago

I'm not sure who would use aurelia-dialog without those default resources, but these new changes is the spirit of the new API 👍

fkleuver commented 6 years ago

The arrow import syntax is unquestionably a huge improvement over PLATFORM.moduleName. You ought to double check that this also works properly with webpack.

Personally not a huge fan of default exports, any particular reason for that?

StrahilKazlachev commented 6 years ago

I hoped for more feedback 😅 @bigopon for me the value of aurelia-dialog has never been in the resources but in the infrastructure - DialogService, DialogController, etc. At work we did use the default resources for short time, but then did our own - corporate look & feel, additional integration functionality(specific to our use cases), ... Also the ability not to register the resources was there from the start.

@fkleuver I was under the impression that using export default for resources was the way to go, but now I can't find any code using it and I'm confused. @bigopon I do remember there was a discussion around () => import('resources/my-resource') about using the default export - don't remember the context though(router, templating, framework). Did it never progress?