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

Improvements to Extensions definition API inside Controllers #180

Closed
jsimck opened this issue Apr 1, 2022 · 1 comment · Fixed by #254
Closed

Improvements to Extensions definition API inside Controllers #180

jsimck opened this issue Apr 1, 2022 · 1 comment · Fixed by #254
Labels
enhancement New feature or request ima@18

Comments

@jsimck
Copy link
Contributor

jsimck commented Apr 1, 2022

Current state

Right now if I want to link extension to concrete page Controller, I need to get an instance of it (usually through $dependencies -> constructor -> instance variable) and call addExtension in init method.

import { AbstractController } from '@ima/core';
import GalleryExtension from 'app/component/gallery/GalleryExtension';

export default class PostController extends AbstractController {
  static get $dependencies() {
    return [GalleryExtension];
  }

  constructor(galleryExtension) {
    this._galleryExtension = galleryExtension;
  }

  init() {
    this.addExtension(this._galleryExtension);
  }
}

This brings a generous amount of boilerplate while currently there's no easy way to define a set of extensions which should be used on multiple controllers, without their explicit definition or some other helper that calls the addExtension behind the scenes.

API Extension proposals

I propose additional 2 methods that can be used along with the original API. These try to minimize the boilerplate needed, focus on better readability and reusability.

1. $extensions

The first addition is in a form of new $extensions static getter in the Controllers. This would work the same way as the current $dependencies (e.g. the instances are handled by the ObjectContainer), while eliminating the usually not-needed boilerplate.

import { AbstractController } from '@ima/core';
import GalleryExtension from 'app/component/gallery/GalleryExtension';

export default class PostController extends AbstractController {
  static get $extensions() {
    return [GalleryExtension];
  }
}

this would also simplify the definition of global extension stacks:

import { AbstractController } from '@ima/core';
import { GlobalExtensions } from 'app/config/extensions';
import GalleryExtension from 'app/component/gallery/GalleryExtension';

export default class PostController extends AbstractController {
  static get $extensions() {
    return [...GlobalExtensions, GalleryExtension];
  }
}

2. Extensions defined in the router itself

The Extensions definition would take place while defining the route along with it's view and controller:

import HomeController from 'app/page/home/HomeController';
import HomeView from 'app/page/home/HomeView';
import GalleryExtension from 'app/component/gallery/GalleryExtension';

export let init = (ns, oc, config) => {
  const router = oc.get('$Router');

  router
    .add('home', '/', HomeController, HomeView, {
      extensions: [GalleryExtension]
    })
}

The same benefits as with the first solution apply here.

Both API extensions should work along with the current implementation (unless there's no way to leave them both in the code).

@jsimck jsimck added the enhancement New feature or request label Apr 1, 2022
@jsimck jsimck added the ima@18 label Apr 12, 2022
@jsimck
Copy link
Contributor Author

jsimck commented Apr 12, 2022

Would be cool if both options also accept array of extension classes. (This could be used to bind array of global extension to certain alias and only use this alias)

@jsimck jsimck linked a pull request Aug 28, 2022 that will close this issue
@jsimck jsimck closed this as completed Sep 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ima@18
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant