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

Added dynamic namespacing to mapState, mapGetters, mapActions #1048

Closed
wants to merge 5 commits into from

Conversation

Stumblor
Copy link

@Stumblor Stumblor commented Nov 6, 2017

Based on this issue #863, I was also struggling to apply a dynamic namespace to a component that could be used anywhere in the component tree. To support the namespace being applied to a component via the props, I added functionality for the namespace to be supplied as a function.

Example usage:

    var namespace_def = "settings/someModule/";

    module.exports = {
        props: [ "namespace" ],
        computed: mapState((self) => self.namespace || namespace_def, [
            'options', 
            'items'
        ]),
        methods: mapActions((self) => self.namespace || namespace_def, [
            'SomeAction', 
            'AnotherAction'
        ])
     };

@ktsn
Copy link
Member

ktsn commented Nov 7, 2017

IMO, it would be better to use function-style mapper described on this comment instead of introducing dynamic namespace.
Because the latter declares the component computed/methods dynamically. It would make harder to type check statically and grasp the component behavior.

@Stumblor
Copy link
Author

Stumblor commented Nov 7, 2017

That's true, but it's at the expense of a more verbose declaration. I can see this becoming more of a problem will large numbers of methods / properties.

Couldn't the type checking be done after the function was resolved?

@Stumblor
Copy link
Author

Stumblor commented Nov 9, 2017

Ok - I'm coming around more to this way of thinking. Going to close this now.

@Stumblor Stumblor closed this Nov 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants