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

Remove default export of a 404.html.html page #36857

Conversation

mdubourg001
Copy link

fixes #36855

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • Integration tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running yarn lint

Comment on lines -421 to 425
if (
!options.buildExport &&
!exportPathMap['/404'] &&
!exportPathMap['/404.html']
) {
exportPathMap['/404'] = exportPathMap['/404.html'] = {
if (!options.buildExport && !exportPathMap['/404']) {
exportPathMap['/404'] = {
page: '/_error',
}
}
Copy link
Contributor

@SukkaW SukkaW May 12, 2022

Choose a reason for hiding this comment

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

IMHO it is not the correct fix.

The code here is designed to work like this:

  • When you specify a custom 404 in exportPathMap:
    • Next.js shouldn't overwrite your custom 404 export alias.
  • When you don't specify a custom 404 in exportPathMap (when, and only when neither /404 nor /404.html exists in exportPathMap):
    • When trailingSlash is disabled (which is by default):
      • only 404.html should be exported (which could be used to render https://example.com/404).
      • ['/404', '/404.html'] will be deduped to /404 after path normalization
    • When trailingSlash is enabled:
      • should export 404/index.html instead (which could be used to render https://example.com/404/).
      • A 404.html should also be exported for backward compatibility with GitHub Pages, GitLab Pages, Cloudflare Pages, Netlify, etc.
      • So, /404 would export /404/index.html and /404.html would export /404.html.

You could take a look at export/worker.ts to see how it handles the 404 (check the isBultinPaths part). Also, an integration test case to verify your fix would be appreciated.

@SukkaW
Copy link
Contributor

SukkaW commented May 14, 2022

The PR is superseded by #36910

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Closing as this should be resolved by #36910 as noted above. Thanks for the PR, though!

@ijjk ijjk closed this May 18, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

next export with exportPathMap in next.config.js creates a duplicate 404.html.html
3 participants