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: use getStaticPaths with PageRouteFragment #253

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jurgenbelien
Copy link
Contributor

@jurgenbelien jurgenbelien commented Mar 8, 2025

Changes

  • Create reusable lib for getting localized slug
  • Combine nesting PageRoute fragments in one file
  • Remove option to dedupe grapqhl fragments (so that they can be imported)
  • Support passing a fragment directly into the datocmsCollection for better typed getStaticPaths

How to test

  1. Open preview link
  2. Navigate to documentation
  3. Drill down in documentation
  4. Verify that all pages work
  5. Inspect the pipeline
  6. See that it passes
  7. Try it out with a new route

Checklist

  • I have performed a self-review of my own code
  • I have made sure that my PR is easy to review (not too big, includes comments)
  • I have made updated relevant documentation files (in project README, docs/, etc)
  • I have added a decision log entry if the change affects the architecture or changes a significant technology
  • I have notified a reviewer

@jurgenbelien jurgenbelien force-pushed the refactor/static-paths branch from 7640530 to d429258 Compare March 8, 2025 15:37
Copy link

cloudflare-workers-and-pages bot commented Mar 8, 2025

Deploying head-start with  Cloudflare Pages  Cloudflare Pages

Latest commit: cf7bdbb
Status: ✅  Deploy successful!
Preview URL: https://2a65d321.head-start.pages.dev
Branch Preview URL: https://refactor-static-paths.head-start.pages.dev

View logs

@@ -34,7 +34,6 @@ module.exports = {
* @see https://github.com/Tonel/typescript-type-generation-graphql-example/blob/2d43584b1d75c9086c4ddd594a6b2401a29b0055/graphql.config.yml#L11-L23
*/
config: {
dedupeFragments: true,
Copy link
Contributor Author

@jurgenbelien jurgenbelien Mar 8, 2025

Choose a reason for hiding this comment

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

This was all that was necessary to be able to import full fragments into the datocmsCollection from @lib/datocms/types.ts. After npm run prep, that file will have all fragments used in the project as template strings, with all their nested fragments injected, ie:

export const PageRoute = gql`
fragment PageRoute on PageRecord {
  ...PageRouteFields
  ...PageRouteParent
}
${PageRouteFields}
${PageRouteParent}`; 

(note that I cleaned up the indentation)

Comment on lines +19 to +23
declare module '*.fragment.graphql' {
import { DocumentNode } from 'graphql';
const value: DocumentNode;
export default value;
}
Copy link
Contributor Author

@jurgenbelien jurgenbelien Mar 8, 2025

Choose a reason for hiding this comment

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

This allows for importing the fragments into code. Note the difference in export statement between *.query.graphql and *.fragment.graphql.

query: parse(/* graphql */`
query ${collection}Meta {
records: all${collection}(first: 1) { __typename }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to get the right __typename, fetch one record from the collection.

@jurgenbelien jurgenbelien force-pushed the refactor/static-paths branch from d429258 to cf7bdbb Compare March 8, 2025 15:57
: fragment;
const { definitions } = fragmentDocument;
const fragmentDefinition = definitions
.find((definition): definition is FragmentDefinitionNode =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

const recordsPerPage = 100; // DatoCMS GraphQL API has a limit of 100 records per request
const totalPages = Math.ceil(meta.count / recordsPerPage);
const records: CollectionType[] = [];
const fragmentDocument = typeof fragment === 'string'
? parse(`fragment InlineFragment on ${type} { ${fragment} }`)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create new fragment to maintain support for passing a string to argument fragment


for (let page = 0; page < totalPages; page++) {
const data = await datocmsRequest({
query: parse(/* graphql */`
${print(fragmentDocument)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Insert fragment definition from fragmentDocument, which is either the fragment passed from an import from @lib/datocms/types.ts or the one created from a string;

query All${collection} {
${collection}: all${collection} (
first: ${recordsPerPage},
skip: ${page * recordsPerPage}
) {
${fragment}
...${fragmentDefinition?.name?.value}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use said fragment

@@ -29,36 +30,60 @@ afterEach(() => {
afterAll(() => server.close());

describe('datocmsCollection:', () => {
test('should successfully fetch a non-paginated collection', async () => {
const fragment = parse(/* graphql */`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test all functions with passing a parsed fragment by default

}
`);

test('supports passing fragment as a string', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test if passing a string keeps working

data: {
meta: { count: totalRecords },
records: [{ __typename: 'MyMockRecord' }],
} satisfies CollectionInfo,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sure the payload for the mocked service is the same type as the expected return value

const pageSlugs = {
en: 'example-page',
nl: 'voorbeeld-pagina'
} as Record<string, string> as Record<SiteLocale, string>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sure that this test scenario works, even when the SiteLocale definition generated from a DatoCMS instance might have completely different locales

Comment on lines +6 to +13
type LocalizedSlugs = {
_allSlugLocales?:
| {
locale?: SiteLocale | null;
value?: string;
}[]
| null;
};
Copy link
Contributor Author

@jurgenbelien jurgenbelien Mar 8, 2025

Choose a reason for hiding this comment

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

A lot of null and undefined in here, but it is important that these definitions have an exact overlap with types from DatoCMS.

}
}
`,
fragment,
Copy link
Contributor Author

@jurgenbelien jurgenbelien Mar 8, 2025

Choose a reason for hiding this comment

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

Congratulations to getting to the end. This is the actual feature!

@jurgenbelien jurgenbelien changed the title refactor: use getStaticPaths with PageRouteFragment feat: use getStaticPaths with PageRouteFragment Mar 8, 2025
@jurgenbelien jurgenbelien requested review from jbmoelker and decrek March 8, 2025 16:15
@jurgenbelien jurgenbelien marked this pull request as ready for review March 8, 2025 16:16
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.

1 participant