thepassle / app-tools

125 stars 8 forks source link

shouldNavigate seems to need a name that's not in the docs #27

Closed davie-robertson closed 2 months ago

davie-robertson commented 2 months ago

Loving these tools, their a real productivity boost, so thanks @thepassle

I'm trying to put a guard on loading a page/element and according to the docs I should just be

        {
            path: `${baseURL}profile`,
            title: 'Profile',
            plugins: [
                lazy(() => import('./pages/user-profile/user-profile.js')),
                {
                    shouldNavigate: (context) => ({
                        condition: () => false,
                        redirect: '/',
                    }),
                    beforeNavigation: (context) => {},
                    afterNavigation: (context) => {},
                },
            ],
            render: () => html`<user-profile></user-profile>`,
        },

but vscode is telling me: Property 'name' is missing in type '{ shouldNavigate: (context: Context) => { condition: () => false; redirect: string; }; beforeNavigation: (context: Context) => void; afterNavigation: (context: Context) => void; }' but required in type 'Plugin'.

also, in the page that I'm exporting the route config to I have a this.user which I am storing in a lit context object, am I able to access that within the route config, or do I need to subscribe to that in the router.js that does the export?

thepassle commented 2 months ago

Yes, all plugins are required to have a name property, also inline plugins. I understand that's not entirely clear from the docs though. Would you be willing to create a PR to the docs to improve them?

It's not entirely clear to me what you mean with the second point, can you elaborate/show some code?

davie-robertson commented 2 months ago

Happy to update the docs, although lazy() is a plugin and that doesn't seem to need a name?

Can/should these be defined at the level of

export const router = new Router({
plugins:[],

...and if so does that mean I can access them (the route guards) via each of my routes by using the name in the plugins other routes?

As for my other question, I have an app-index.js file that imports the router config from router.js, app-index.js has a this.user var that gets set when firebase auth triggers:

@provide({ context: userContext })
user: User | null = null;

app-index.js does an import { router } from './router.js'; which contains all the routes and configs (as per the pwa-starter template) and I would like my guard setup based on the/a user being logged in. I understand that's a case of setting the shouldNavigate Condition, but I'm not sure if I have to @consume(({ context: userContext}) from within router.js to make that work.

Apologies in advance if these are silly questions

thepassle commented 2 months ago

The lazy plugin also has a name: https://github.com/thepassle/app-tools/blob/00d4f78a0e4cac5081636989f34e53f0981eca5e/router/plugins/lazy.js#L8

davie-robertson commented 2 months ago

I'll figure this out and update the docs, but I need some help with creating a shouldNavigate plugin, sorry if this isn't the right place, happy to take this to the lit discord channel.

I've now made a authGuardPlugin that's

import { auth } from './libs/firebase-app.js';
import { Plugin } from '@thepassle/app-tools/router.js';

export const authGuardPlugin: Plugin = {
    name: 'auth-guard',
    shouldNavigate: () => ({
        condition: () => !!auth.currentUser,
        redirect: '/',
    }),
};

and I am applying that to my route:

{
    path: `${baseURL}profile`,
    title: 'Profile',
    plugins: [
        authGuardPlugin,
        lazy(() => import('./pages/user-profile/user-profile.js')),
    ],
    render: () => html`<user-profile></user-profile>`,
}

but it fails every time.

Could this just be the asynchronous nature of firebase auth?

thepassle commented 2 months ago

I would expect something like that could/should work, but its hard to say without knowing what firebase-app.js looks like

davie-robertson commented 2 months ago

I got it working as follows - might be useful for someone else that uses firebase auth and wants to use the user object with shouldNavigate

import { auth } from './libs/firebase-app';
import { Plugin } from '@thepassle/app-tools/router.js';
import { onAuthStateChanged } from 'firebase/auth';

// Helper function to check auth state
const checkAuthState = () => {
  return new Promise<boolean>((resolve) => {
    // Ensure the promise resolves to a boolean
    onAuthStateChanged(auth, (user) => {
      resolve(!!user); // Resolve with true if user exists, false otherwise
    });
  });
};

export const authGuardPlugin: Plugin = {
  name: 'auth-guard',
  shouldNavigate: (_context) => {
    return {
      redirect: '/',
      condition: () => async () => {
        // Return a function that returns a Promise<boolean>
        const isAuthenticated = await checkAuthState();
        return isAuthenticated;
      },
    };
  },
};

firebase-app simply initialises firebase and exports a few objects for use across my app, one them being a firebase auth object