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

[nextra]: refactor loaders.ts, provide type for PageOpts in loader #579

Merged
merged 3 commits into from Jul 30, 2022

Conversation

dimaMachina
Copy link
Collaborator

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Jul 24, 2022

🦋 Changeset detected

Latest commit: dfa509e

The changes in this PR will be included in the next version bump.

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

@vercel
Copy link

vercel bot commented Jul 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
nextra-theme-docs-dev ✅ Ready (Inspect) Visit Preview Jul 30, 2022 at 10:44AM (UTC)
1 Ignored Deployment
Name Status Preview Updated
nextra ⬜️ Ignored (Inspect) Jul 30, 2022 at 10:44AM (UTC)

Comment on lines 125 to 174
interface LayoutProps {
filename: string
pageMap: PageMapItem[]
meta: Meta
titleText: string | null
headings?: Heading[]
timestamp?: number
}

const Content: React.FC<LayoutProps> = ({
const Content = ({
filename,
pageMap,
meta,
titleText,
headings,
timestamp,
children
}) => {
}: PageOpts & { children: ReactNode }): ReactElement => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

don't need to specify props since we'll receive all PageOpts anyway

const createLayout = (opts: PageOpts, config: DocsThemeConfig) => {

<Content {...opts}>{page}</Content>

Copy link
Collaborator

@promer94 promer94 Jul 24, 2022

Choose a reason for hiding this comment

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

could use PropsWithChildren<PageOpts> here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PropsWithChildren has optional children, and like this his is required

Comment on lines 256 to 288
const extendedConfig = Object.assign({}, defaultConfig, config, opts)

const createLayout = (opts: PageOpts, config: DocsThemeConfig) => {
const extendedConfig = {
...defaultConfig,
...config,
unstable_flexsearch: opts.unstable_flexsearch
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was a mistake, we don't need to copy all PageOpts in ThemeConfigContext, but only unstable_flexsearch

Comment on lines 174 to 175
const pageOpts: Omit<PageOpts, 'pageMap' | 'titleText'> = {
filename: slash(filename),
route: slash(route),
meta,
headings,
timestamp,
unstable_flexsearch,
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now pageOpts is type safe

codeblocks: boolean
}
titleText?: string
headings: (Heading & { value: string })[]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

before headings was marked optional, but this is false, headings always exist

compiler.data('headings', [])

Comment on lines 188 to 193
filename: "${slash(filename)}",
route: "${slash(route)}",
meta: ${JSON.stringify(data)},
pageMap: __nextra_pageMap__,
titleText: typeof titleText === 'string' ? titleText : undefined,
headings: ${JSON.stringify(headings)},
${timestamp ? `timestamp: ${timestamp},\n` : ''}
${
unstable_flexsearch
? `unstable_flexsearch: ${JSON.stringify(unstable_flexsearch)}`
: ''
}
...${JSON.stringify(pageOpts)}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now it's looks much friendly

@dimaMachina dimaMachina marked this pull request as ready for review July 24, 2022 12:22
@shuding
Copy link
Owner

shuding commented Jul 29, 2022

Could you resolve the conflicts?

Here're the build errors btw:

CleanShot 2022-07-29 at 15 38 17@2x

@dimaMachina
Copy link
Collaborator Author

dimaMachina commented Jul 29, 2022

@shuding sure, I'm planning to resolve conflicts on all PRs :)

meta: Meta
meta: PageOpts['meta']
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was a mistake, it's not meta.json#theme but frontmatter data!

Comment on lines -105 to +106
export interface Meta extends PageTheme {
title: string
}

type Meta = Record<string, any>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As I said Meta is frontmatter data and not meta.json#theme

@dimaMachina dimaMachina marked this pull request as ready for review July 29, 2022 18:18
@dimaMachina dimaMachina requested a review from shuding July 29, 2022 18:18
@dimaMachina dimaMachina changed the title chore(nextra/blog/docs): improve loaders types [nextra]: refactor loaders.ts, provide type for PageOpts in loader Jul 30, 2022
Comment on lines -138 to -153
if (repository.isShallow() && !wasShallowWarningPrinted) {
if (process.env.VERCEL) {
console.warn(
'[nextra] The repository is shallow cloned, so the latest modified time will not be presented. Set the VERCEL_DEEP_CLONE=true environment variable to enable deep cloning.'
)
} else if (process.env.GITHUB_ACTION) {
console.warn(
'[nextra] The repository is shallow cloned, so the latest modified time will not be presented. See https://github.com/actions/checkout#fetch-all-history-for-all-tags-and-branches to fetch all the history.'
)
} else {
console.warn(
'[nextra] The repository is shallow cloned, so the latest modified time will not be presented.'
)
}
wasShallowWarningPrinted = true
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can be moved outside of loader (inside of IIFE) and wasShallowWarningPrinted variable can be removed

@shuding shuding merged commit 0f4795f into core Jul 30, 2022
@shuding shuding deleted the improve-types-for-loader branch July 30, 2022 14:48
@shuding
Copy link
Owner

shuding commented Jul 30, 2022

Thank you!

tatukoivisto pushed a commit to tatukoivisto/nextra that referenced this pull request Aug 20, 2023
shuding#579)

* chore(nextra/blog/docs): provide types for PageOpts in loader

* yet more readable

* move `shallow cloned` console messages outside of loader
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants