-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
add support for remote [...catchAll] routes #1294
Conversation
🦋 Changeset detectedLatest commit: fff9f6b The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work! Left one thing for discussion
"/getting-started": { | ||
"parser-options": "parser-options", | ||
parser: "parser", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered the items
proposal from #1239? Or supporting /
in keys like:
{
"configs": "configs",
"getting-started": "getting-started",
"getting-started/parser-options": "parser-options",
"getting-started/parser": "parser"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can support both?
Update:
I guess the proposal with items is much nicer and later can be used for a global meta file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shuding after I realised that items
solution couldn't be done because there is a case of existing mdx file and folder of the same name
{
"foo": "Foo", // this is foo.mdx
"foo": { // this is foo folder but this will overwrite foo file key
"title": "Foo",
"type": "Category",
"items": {}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they need to be merged right? It's still 1 item on the sidebar 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they should not be merged so nextra under the hood will know if folder has folder index page or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tricky thing was i18n. If users can choose foo.en.mdx
or foo/index.en.mdx
(they point to the same URL), and when accessing /foo
, the middleware doesn't know if it's foo.en
or foo/index.en
. So in the past I documented it as you'll always have to avoid index.en.mdx
.
But I think overall the middleware based i18n solution is too hacky and I'd like to fix it properly. I have an idea for that and I'll probably start working on it soon!
05526ff
to
acfb648
Compare
e384626
to
1d5884b
Compare
50bb31b
to
45d49d5
Compare
const { listFiles } = require('../../../components/remote-utils') | ||
const { createCatchAllMeta } = require('nextra/catch-all') | ||
|
||
module.exports = async () => { | ||
const files = await listFiles() | ||
return createCatchAllMeta( | ||
// Ensure you didn't forget define some file by providing all paths of remote files | ||
files | ||
.filter(filename => filename.startsWith('website/src/pages/docs/')) | ||
.map(filename => filename.replace('website/src/pages/docs/', '')), | ||
// Next you can override order of your meta files, folders should end with / and be nested | ||
{ | ||
index: 'Introduction', | ||
'getting-started': 'Getting Started', | ||
'getting-started/': { | ||
parser: 'Parser' | ||
} | ||
} | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shuding I still think that nested keys with slashes (I changed the order, now folders should end with slashes, like in .gitignore
) are the most friendly way since less code is written.
With the above helper function, I was able to change the original order of items with a few lines coded
from
to
bb70829
to
e0100e2
Compare
e0100e2
to
1fef24d
Compare
4358801
to
be8ccf7
Compare
Wow amazing result!! Code LGTM! |
support meta keys with `/` fixes update snapshots fix hydration error (caused by extra `;`) correct handle `index` route use `title` package for dynamic meta's add createCatchAllMeta() helper function use createCatchAllMeta in tests fix meta ordering fix tests dry add another example fixes fix folder name link rewrite refactor to use `items` prop for folders simplify better folder renaming remove `node:` prefix
be8ccf7
to
682949e
Compare
* add support for remote `[...catchAll]` routes support meta keys with `/` fixes update snapshots fix hydration error (caused by extra `;`) correct handle `index` route use `title` package for dynamic meta's add createCatchAllMeta() helper function use createCatchAllMeta in tests fix meta ordering fix tests dry add another example fixes fix folder name link rewrite refactor to use `items` prop for folders simplify better folder renaming remove `node:` prefix * Apply suggestions from code review * update snapshots after rebase * add console.log * bundle nextra esm to es2020 * cache getDynamicMeta * add prefer-destructuring * cache result of `globalThis.__nextra_resolvePageMap`
how look like catch all routes for
graphql-eslint
repository fetched from git[...catchAll]
routesimport
statementscollectCatchAll
that adds to pageMap meta files and mdx pagesfor some reason remote mdx wrapped with
<p/>
<RemoteContent />
now acceptcomponents
propscodeHighlight: true
anddefaultShowCopyCode: true
while compiling remote mdx_meta
file withcreateCatchAllMeta()
[hello](/docs/foo)
=>[hello](/docs/v2/foo)
in remote mdx contentsupport meta keys with❗️this no longer needed/