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

feat(builder): support async extendRoutes #7754

Merged
merged 2 commits into from
Jul 22, 2020
Merged

feat(builder): support async extendRoutes #7754

merged 2 commits into from
Jul 22, 2020

Conversation

MartinN3
Copy link

Current behaviour
its impossible to extend routes with APIs

Expected behaviour
Being able to await requests to server that are required for extendingRoutes

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

With this simple change, we are able to extend our nuxt router with async data without need to create custom router.

In documentation is something like this:

router: {
  extendRoutes(routes, resolve) {
    routes.push({
      name: 'PageHierarchyPage',
      path: '/test/test/',
      props: {
        id: 12,
      },
      component: resolve(__dirname, 'pages-extended/PageHierarchyPage/_.vue'),
    });
    sortRoutes(routes);
  }
}

But we might need something like this:

router: {
  extendRoutes: async (routes, resolve) => {
   const dynamicRoutes = await getDynamicRoutes();
   const dynamicRoutesTransformed = dynamicRoutes.map(item => ({
     ...item,
     component: resolve(__dirname, item.component)
   }));
   routes.push(...dynamicRoutesTransformed);
   sortRoutes(routes);

   return routes;
  }
}

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@MartinN3 MartinN3 changed the title nuxt config - await extendRoutes to allow async requests feat: nuxt config - await extendRoutes to allow async requests Jul 21, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2020

Codecov Report

Merging #7754 into dev will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #7754   +/-   ##
=======================================
  Coverage   68.83%   68.83%           
=======================================
  Files          90       90           
  Lines        3821     3821           
  Branches     1034     1034           
=======================================
  Hits         2630     2630           
  Misses        968      968           
  Partials      223      223           
Flag Coverage Δ
#unittests 68.83% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/builder/src/builder.js 90.17% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7aa46f5...4f120a0. Read the comment docs.

@MartinN3 MartinN3 changed the title feat: nuxt config - await extendRoutes to allow async requests feat:(builder) - await extendRoutes to allow async requests Jul 21, 2020
@MartinN3 MartinN3 changed the title feat:(builder) - await extendRoutes to allow async requests feat(builder): - await extendRoutes to allow async requests Jul 21, 2020
@MartinN3 MartinN3 changed the title feat(builder): - await extendRoutes to allow async requests feat(builder): await extendRoutes to allow async requests Jul 22, 2020
@MartinN3
Copy link
Author

Can this idea be even considered good or the whole thing should be scraped? Thanks for insights.

btw:
Some unit test wasn't successful? I can't really see which one and what failed tho.
I'll have to read details of semantic pull requests.

Current behaviour
its impossible to extend routes with APIs

Expected behaviour
Being able to await requests to server that are required for extendingRoutes

With this simple change, we are able to extend our nuxt router with async data without need to create custom router.
@manniL manniL requested a review from a team July 22, 2020 07:49
@@ -420,7 +420,7 @@ export default class Builder {
)
// Only overwrite routes when something is returned for backwards compatibility
if (extendedRoutes !== undefined) {
templateVars.router.routes = extendedRoutes
templateVars.router.routes = await extendedRoutes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think would be better to put await here because result of the promise can be undefined too

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved

@pi0 pi0 merged commit b469f93 into nuxt:dev Jul 22, 2020
@pi0 pi0 changed the title feat(builder): await extendRoutes to allow async requests feat(builder): support async extendRoutes Jul 22, 2020
@pi0 pi0 mentioned this pull request Jul 22, 2020
@gianpesto
Copy link

Thank you for the merge,
i am having troubles wrapping my head around a similiar problem:

i'd like to extend Routes as well after fetching data form an API, but during runtime.

the scenario is the following:

  1. A User navigates to the app (possibly with a specific locale 'en-us')
  2. User has to login using nuxt auth module
  3. routes are being fetched from the Server (can be different for each user, depending on permissions, locale, ecc...)
  4. extend Routes

i am now having one dynamic route that checks the requested route and shoves it into a dynamic component, but somehow it feels wrong to do it that way 🤮

any help would be appreciated

@pi0
Copy link
Member

pi0 commented Aug 18, 2020

Hey @gianpesto maybe opening a dicsussion with more details how are you currently doing stuff? :)

@gianpesto
Copy link

Opened one ☺️

@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants