[MobX 4] Action decorator being non-configurable breaks further decoration

[JabX]

Hello,

I've been successfully using @action decorators along side a global @autobind decorator on the class (from core-decorators) for a while, but since I upgraded to MobX 4 it just stopped working altogether.

The @autobind decorator I use breaks here, because it tries to redefine the property defined by @action there, only because MobX (explicitely) defines the property as non configurable.

Is there a particuliar reason why this behavior was chosen ? I've read on other issues that all decorator stuff was put on hold until the spec finalizes, but the fix looks simple (unless I'm missing something) and it's a pretty major regression for me.

I don't see any other alternative than either:

• request a fix on the other library to stop trying to redefine non configurable properties (forcing me to replace all my @actions by @action.bounds)
• remove every @autobind class decorator and put them on each method instead, along with @action.bound where applicable
• go back to MobX 3

And I'd like to avoid going to such lengths.

Thanks for your time.

[mweststrate]

you don't need autobind when using mobx, just use @action.bound instead :) Op wo 4 apr. 2018 om 20:47 schreef Damien Frikha <[email protected]>:

…

Hello, I've been successfully using @action decorators along side a global @autoBind decorator on the class (from core-decorators <https://github.com/jayphelps/core-decorators>) for a while, but since I upgraded to MobX 4 it just stopped working altogether. The @autoBind decorator I use breaks here <https://github.com/jayphelps/core-decorators/blob/master/src/autobind.js#L44>, because it tries to redefine the property defined by @action there <https://github.com/mobxjs/mobx/blob/master/src/api/actiondecorator.ts#L23>, only because MobX (explicitely) defines the property as non configurable. Is there a particuliar reason why this behavior was chosen ? I've read on other issues that all decorator stuff was put on hold until the spec finalizes, but the fix looks simple (unless I'm missing something) and it's a pretty major regression for me. I don't see any other alternative than either: - request a fix on the other library to stop trying to redefine non configurable properties (forcing me to replace all my @actions by @action.bounds) - remove every @autoBind class decorator and put them on each method instead, along with @action.bound where applicable - go back to MobX 3 And I'd like to avoid going to such lengths. Thanks for your time. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1477>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhOoTYdJo-h6hWG82c_PzZQ2Ji0-4ks5tlRVSgaJpZM4THTv1> .

[JabX]

Well, @action.bound is supposed to be used for actions, which may not always represent all methods of a class, and a global @autobind decorator on a class represents far less noise to me than decorating each and every method (or adding .bound on all actions).

In any case, everybody is free to choose the binding method that works best for him/her, but mine doesn't work anymore in MobX 4 because of a tiny configurable: false, which might probably be unnecessary. If there's a valid reason for it then I guess I'll have to adapt, but if not I'd rather like for it to be changed.

[Strate]

@mweststrate unfortunatelly this is huge break for upgrading from mobx3 to mobx4 :(

UPD also this is not caught by typescript, so this become a real pain :(

[mweststrate]

will address next week Op ma 9 apr. 2018 om 14:48 schreef Artur Eshenbrener < [email protected]>:

…

@mweststrate <https://github.com/mweststrate> unfortunatelly this is huge break for upgrading from mobx3 to mobx4 :( — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1477 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhBpg34H_RXSNdS63kdbJtwpfJqBgks5tm1i3gaJpZM4THTv1> .

[Strate]

Temporary fixed as making middleware module, which exports custom action and action.bound functions, which calls to original mobx function and removes from there configurable property. Middleware module applied to whole project by webpack's NormalModuleReplacementPlugin:

fixMobx.ts:

export * from "mobx"

// TODO Remove after resolve of https://github.com/mobxjs/mobx/issues/1477

import {action as mobxAction} from "mobx"

export const action: typeof mobxAction = function(...args: any[]): any {
    const res = mobxAction.apply(this, args);
    if (typeof res === "object" && "configurable" in res && res.configurable === false) {
        delete res.configurable
    }
    return res;
} as typeof mobxAction

action.bound = function(...args: any[]): any {
    const res = mobxAction.apply(this, args);
    if (typeof res === "object" && "configurable" in res && res.configurable === false) {
        delete res.configurable
    }
    return res;
}

webpack.config.js:

new webpack.NormalModuleReplacementPlugin(/^mobx$/, function(resource) {
    // TODO Remove after resolve of https://github.com/mobxjs/mobx/issues/1477
    // Don't forget to adjust path to fixMobx.ts to your project
    if (resource.contextInfo.issuer !== path.join(srcDir, "web", "lib", "fixMobx.ts")) {
        resource.request = path.join(srcDir, "web", "lib", "fixMobx.ts")
    }
})

[hector]

I am looking forward for this fix. I'm holding upgrade to version 4 until then

[rkuykendall]

Just more data for the team: I use class-autobind-decorator on my classes. Currently this is incompatible with mobx 4.

[mweststrate]

Fixed and released as 4.2.0. Actions are now always configurable and writable.