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

Convert Outlet to use a render prop to describe the output #60

Closed
agubler opened this issue Aug 12, 2018 · 1 comment
Closed

Convert Outlet to use a render prop to describe the output #60

agubler opened this issue Aug 12, 2018 · 1 comment
Labels
breaking change Indicates the issue/pull request would result in a breaking change enhancement New feature or request next Issue/Pull Request for the next major version

Comments

@agubler
Copy link
Member

agubler commented Aug 12, 2018

Enhancement

An Outlet is a basic higher order component which accepts one or more widgets on creation of the outlet.

// separate outlet component file

export const MyOutlet = Outlet({
    index: IndexWidget, 
    main: MainWidget, 
    error: ErrorWidget }, 'outlet-name'):

The current implementation is problematic for typings when there are multiple widgets specified as the Outlet's properties becomes a union type of all the widget's properties passed to the Outlet. This is not correct, as you may need to pass different values for the same property depending on the match type scenario. Another side affect means that widgets could receive extra properties that were intended for a different widget.

There are at least a couple of potential options:

Option 1

Move from a HOC to a component that accepts a render property to describe the outlets output. This will allow consumers to explicitly pass the properties to each of the widgets and give more control to the consumer to create customised logic on which widgets to display.

// inside your widget
render() {
    return v('div', [
        w(Outlet, { 
            to: 'outlet-name', 
            renderer: (matchDetails) => {
                // match details contains the `matchType`, `params` and `queryParams`
                return w(MyWidget, { foo: 'bar', id: matchDetails.id });
            }
        })
    ]);
}

Options 2

Keep with a HOC to a component but one that accepts a render function to describe the outlets output. This will also allow consumers to explicitly pass the properties to each of the widgets and give more control to the consumer to create customised logic on which widgets to display.

interface MyOutletProperties {
    foo: string;
}

const MyOutlet = Outlet<MyOutletProperties>((properties, outletProps) => {
     if (outletProps.isError()) {
        return w(ErrorWidget, {});
     }
     if (outletProps.isExact()) {
         return w(ExactMatchWidget, {});
     }

     // return undefined when no widget is needed to be rendered based on the match type.
     return undefined;
}, 
{ outlet: 'foo' })
@agubler agubler added enhancement New feature or request breaking change Indicates the issue/pull request would result in a breaking change next Issue/Pull Request for the next major version labels Aug 12, 2018
@agubler
Copy link
Member Author

agubler commented Aug 13, 2018

I prefer option 1, a HOC for an Outlet seems overkill - moving to a standard widget reduces the complexity as there is no need to define a separate "outlet properties" interface because all the defining widgets' state and properties will be available in the render function.

Additionally it reduces the boilerplate required for outlets and removes a layer of indirection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Indicates the issue/pull request would result in a breaking change enhancement New feature or request next Issue/Pull Request for the next major version
Projects
None yet
Development

No branches or pull requests

1 participant