VulcanJS / Vulcan

🌋 A toolkit to quickly build apps with React, GraphQL & Meteor
http://vulcanjs.org
MIT License
7.98k stars 1.88k forks source link

withAccess HoC #1806

Closed muninn9 closed 6 years ago

muninn9 commented 6 years ago

What's the deal with the withAccess HoC? I'm not seeing it anywhere in the core code yet it's in the docs. When I try using it I get this error:

TypeError: hoc[0] is not a function at component.hocs.map.hoc (packages/vulcan:lib/lib/modules/components.js:53:63)

SachaG commented 6 years ago

Sorry, it's only on the devel branch currently. I guess I should've mentioned that in the docs.

muninn9 commented 6 years ago

I also had an issue using the manual redirect solution (as described in the docs) with nested components and one of the solutions I came up with is described below. I thought it might be worth including in the docs (or somewhere) as another authentication method.

routes.js

addRoute({ name: 'MembersOnly', componentName: 'MembersOnly'  });

addRoute({
  name: 'account',
  path: '/account',
  component: Account,
  layoutName: "MainLayout"
}, 'MembersOnly');

MembersOnly component

import React from 'react';
import { withRouter } from 'react-router';
import { registerComponent, withCurrentUser } from 'meteor/vulcan:core';

class MembersOnly extends React.Component {
    constructor(props) {
        super(props);

        if(!props.currentUser) {
            props.router.push('/login');
        }
    }

    render() {
        if (this.props.currentUser) {
          return this.props.children
        } else {
          return null
        }
    }
}

registerComponent('MembersOnly', MembersOnly, withCurrentUser, withRouter);

This way the authentication happens at a higher level so you only have to write it once. For instance, all members only components are children of the 'MembersOnly' component.

SachaG commented 6 years ago

I'm not sure I understand your method, do you mean you wanted all sub-routes to have the same access control?

muninn9 commented 6 years ago

Yeah, it's basically what you have described in the docs but it's happening at the router level rather than within each individual component so if there were multiple components that required the same access they would all be sub-routes of one component.

SachaG commented 6 years ago

Oh ok, I didn't get it at first, that's pretty smart! I don't know if I want to give people too many methods of doing the same thing in the docs though, as it can get confusing. Or maybe as an example in the section about sub-routes?

muninn9 commented 6 years ago

When I was reading the docs I was hoping to find a solution like this in the routing section, but that's just me. Flow Router describes "group routes" in their docs which is an analogous pattern.

SachaG commented 6 years ago

OK, I'll accept a PR to add that pattern to the routing section.

SachaG commented 6 years ago

https://github.com/VulcanJS/vulcan-docs btw :)

muninn9 commented 6 years ago

There's just one issue: it only works when the sub-route uses the actual component via importing rather than the component name via registerComponent

SachaG commented 6 years ago

Oh weird. I guess we'd better leave it out of the docs until we can figure that one out then, it might be confusing to have multiple approaches to component importing.

SachaG commented 6 years ago

Oh actually I wonder if it's because of #1813

muninn9 commented 6 years ago

That's definitely in the right direction though it didn't work for me (shows "Not found"). This did however:

addRoute({ name: 'MembersOnly', componentName: 'MembersOnly' }); addRoute({ name: 'account', path: '/account', component: getComponent("Account"), <-----added getComponent here layoutName: "MainLayout" }, 'MembersOnly');

So it looks like if #1813 was refined a bit more it would work.

SachaG commented 6 years ago

cc @Sebi55 :)

sebastiangrebe commented 6 years ago

For me your version works when using componentName on the second route:

addRoute({ name: 'MembersOnly', componentName: 'MembersOnly' });
addRoute({
    name: 'account',
    path: '/account',
    componentName: "Account"
}, 'MembersOnly');

Are you sure you have the changes from the devel branch to test it? The routing should work anyway the only thing which could not work would be getting the component.

I personally got my routes for exmaple like that and that works perfectly fine.

import { addRoute } from 'meteor/vulcan:core';
import { Components  } from 'meteor/vulcan:core';
addRoute({ name: 'home', path: '/', componentName: 'Home' });
addRoute({ name: 'logout', path: '/logout', componentName: 'logout' });
addRoute({ name: 'app', path: '/app', componentName: 'app' });
addRoute({ name: 'app.list', path: '/app/list', componentName: 'userList' },'app');
addRoute({ name: 'app.user', path: '/app/user', componentName: 'userProfile' },'app');

I can remove the path from app without any problems.