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

feat(resources): register resources by class #354

Conversation

Projects
None yet
3 participants
@StrahilKazlachev
Copy link
Contributor

commented Jul 28, 2018

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:

  • remove the reexports of the resources from the plugin entry point
  • don't import them statically anywhere in the mandatory modules
    • use import() to conditionally import them in the configuration module

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

feat(resources): register resources by class
BREAKING CHANGE - default resources are no longer reexported, need to be explicitly included when bundling
@bigopon

This comment has been minimized.

Copy link
Member

commented Jul 28, 2018

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

This comment has been minimized.

Copy link
Member

commented Jul 31, 2018

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

This comment has been minimized.

Copy link
Contributor Author

commented Aug 3, 2018

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?

@StrahilKazlachev StrahilKazlachev force-pushed the StrahilKazlachev:feat/resources/register-by-class branch from 443a7ea to 0d09d2c Aug 11, 2018

@StrahilKazlachev StrahilKazlachev force-pushed the StrahilKazlachev:feat/resources/register-by-class branch from 0d09d2c to 9a4b29c Aug 11, 2018

@StrahilKazlachev StrahilKazlachev merged commit ff55c80 into aurelia:v2.x Aug 11, 2018

2 checks passed

WIP ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@StrahilKazlachev StrahilKazlachev deleted the StrahilKazlachev:feat/resources/register-by-class branch Aug 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.