fix!: remove catch-all DSG redirect and handle discretely #334
Conversation
✅ Deploy Preview for netlify-plugin-gatsby-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Looks good, but let's hold off merging this until netlify/gatsby-plugin-netlify#102 lands
plugin/src/helpers/config.ts
Outdated
| if (!hasPlugin(gatsbyConfig.plugins, 'gatsby-plugin-netlify')) { | ||
| if (hasPlugin(gatsbyConfig.plugins, 'gatsby-plugin-netlify')) { | ||
| if ( | ||
| // prettier-ignore |
There was a problem hiding this comment.
FYI this ignore is just to avoid tripping over the max-lines lint rule!
There was a problem hiding this comment.
Why does it need a prettier ignore for that? Isn't that an eslint rule?
There was a problem hiding this comment.
the file was under 150 lines until prettier formatted that 1 line onto 3. I tried ignoring prettier to see if it passed the eslint rules and it did, so I just kept it there. I can swap it for the eslint-ignore if you think it makes more sense?
There was a problem hiding this comment.
Yeah, just do the eslint ignore for max lines for the whole file
plugin/src/helpers/files.ts
Outdated
| name: string, | ||
| version: string, | ||
| ): Promise<boolean> { | ||
| const packagePath = findModuleFromBase({ |
There was a problem hiding this comment.
I appreciate the desire to reuse the existing funciton, but I think in this situaiton calling require.resolve directly with ${packagePath}/package.json is probably better than using that
There was a problem hiding this comment.
fair point, have amended
Summary
BREAKING CHANGE: DSG pages are now discretely redirected to the handler function, rather than via a catch-all redirect. This PR removes the catch-all redirect and moves DSG handling to netlify/gatsby-plugin-netlify, making that plugin a requirement for DSG pages.
This change ensures that 404 pages aren't unnecessarily handled via SSR and ensures they work when a site has functions but no SSR/DSG bundle.
Test plan
Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #330
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.