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

Allow mixins to seamlessly override existing methods #304

Open
konne opened this issue Nov 8, 2017 · 4 comments
Open

Allow mixins to seamlessly override existing methods #304

konne opened this issue Nov 8, 2017 · 4 comments

Comments

@konne
Copy link

konne commented Nov 8, 2017

If I add a mixin I have no chance to make something like an "inserOrUpdate".

It would be great to have a section there I can add function and enigma gives me
as a parameter if this function exists or not.
So that I can decide in my mixin what to do.

For example. We develop an mixin like #272 and a later engine version delivers even on .app an getProperties.
With the current enigma.js we have no chance to write a generic working mixin.

@marcusoffesson
Copy link
Contributor

Good point!
One way to solve this would be to remove the mixin.extend and mixin.override properties and modify the signature of all mixin methods to take baseFn as its first argument.

For example:

const mixin = {
  types: ['Doc'],
  init(args) {},
  methods: {
    method1(baseFn, arg1, arg2) {
      if (baseFn) {
        // base function exists (overrides existing method)
      } else {
        // no base function (extends the generated api)
      }
    } 
  }

Is this something that would work for you?

@konne
Copy link
Author

konne commented Nov 10, 2017

Hi @marcusoffesson,
this should work, but this mixin.methods is at least at the moment not documented.
If this is already in the product please extend https://github.com/qlik-oss/enigma.js/blob/2fe4538cdeeb6d2a174cd78cf96906de91bcfce5/docs/api.md#mixins

@marcusoffesson
Copy link
Contributor

Hi @konne!
My previous comment should be read as a design proposal and not as something that already exists. Also, we have to consider other things, such as backwards compatibility, etc.

@konne
Copy link
Author

konne commented Nov 13, 2017

Hi @marcusoffesson,
okay, the design proposal sounds great, so this would perfectly work.

@peol peol changed the title Feature Request: Mixin -> override or extend with flag Allow mixins to seamlessly override existing methods Nov 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants