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

After the initial render, any direct url visits to pages result in 404s #5620

Closed
vjpr opened this issue Nov 7, 2018 · 8 comments · Fixed by #6635
Closed

After the initial render, any direct url visits to pages result in 404s #5620

vjpr opened this issue Nov 7, 2018 · 8 comments · Fixed by #6635
Assignees

Comments

@vjpr
Copy link

vjpr commented Nov 7, 2018

Bug report

Describe the bug

in require.js#getPagePath, the pages-manifest.json file is read using:

const pagesManifest = require(join(serverBuildPath, PAGES_MANIFEST))

But every time a new on-demand entry is added to the build, the pages-manifest.json file changes.

Using require though returns a cached version.

This causes requirePage to say module could not be found, which causes a 404.

Fix

Strangely, deleting the require.cache entry didn't resolve it.

delete require.cache[join(serverBuildPath, PAGES_MANIFEST)]

But using the following resolved my issue:

  const pagesManifest = JSON.parse(require('fs').readFileSync(join(serverBuildPath, PAGES_MANIFEST), 'utf8'))

System information

  • OS: [e.g. macOS, Windows] macOS
  • Browser (if applies) [e.g. chrome, safari] Chrome
  • Version of Next.js: [e.g. 6.0.2] v7.0.2-canary.20 / b46d7e8
@timneutkens
Copy link
Member

timneutkens commented Nov 7, 2018

I wonder what's happening here, as it's definitely working fine, all tests are passing. Are you using a custom server / custom webpack config? We already flush the require.cache in the nextjs-require-cache-hot-reloader.js webpack plugin, and that has been working pretty well so far 🤔

@vjpr
Copy link
Author

vjpr commented Nov 7, 2018

Are you using a custom server / custom webpack config?

Yeh, its quite custom.

We already flush the require.cache in the nextjs-require-cache-hot-reloader.js webpack plugin, and that has been working pretty well so far

Ah, this is probably the source of the problem. I will dig into it.

@vjpr
Copy link
Author

vjpr commented Nov 7, 2018

Don't know if it is a cause but nextjs-require-cache-hot-reloader.js exports class called ChunkNamesPlugin. A typo I suspect. There is also another plugin called ChunkNamesPlugin.

@timneutkens
Copy link
Member

Don't know if it is a cause but nextjs-require-cache-hot-reloader.js exports class called ChunkNamesPlugin. A typo I suspect. There is also another plugin called ChunkNamesPlugin.

Changed the name, good catch! it won't affect this issue though

@vjpr
Copy link
Author

vjpr commented Nov 7, 2018

delete require.cache[X] is not working for me inside getPagePath.

So seems like a require hook or something is preventing this - probabably something on my end.

@vjpr
Copy link
Author

vjpr commented Nov 7, 2018

Found the problem.

The issue was to do with my output dir being inside a symlinked dir. Node caches realpaths, but NextJsRequireCacheHotReloader doesn't resolve realpaths before clearing from cache.

join(serverBuildPath, PAGES_MANIFEST)

/xxx/packages/xxx-symlink/pages-manifest.json

require.resolve(join(serverBuildPath, PAGES_MANIFEST))

/xxx/packages/xxx/pages-manifest.json

Fix

NextJsRequireCacheHotReloader must fs.realpath before clearing cache.

function deleteCache (path: string) {
  path = require('fs').existsSync(path) ? require('fs').realpathSync(path) : path
  delete require.cache[path] 
}

realPath on non-existent file will throw.

I wonder how this will effect performance though.

We should cache realpaths perhaps.

@vjpr
Copy link
Author

vjpr commented Nov 8, 2018

@timneutkens Would you accept a PR for the fix I suggest above?

@timneutkens
Copy link
Member

Sounds like the correct fix, however you don't have to use the sync version, the tap is async.

bhongy pushed a commit to bhongy/next.js that referenced this issue Nov 9, 2018
This fixes vercel#5260 by making sure that `index.js` has `getInitialProps` defined on the page exported component, not the child component.

When fixing that, I uncovered an issue where the server side rendered HTML did not match the clientside HTML, so I reworked _app.js to use the `i18nextprovider` component which has props to hydrate the initial data (for SSR), and makes sure the correct i18n instance is passed to all child components through context.

Before:
```html
<!DOCTYPE html>
<html>
   <head>
      <meta charSet="utf-8" class="next-head"/>
      <link rel="preload" href="/_next/static/development/pages/index.js" as="script"/>
      <link rel="preload" href="/_next/static/development/pages/_app.js" as="script"/>
      <link rel="preload" href="/_next/static/development/pages/_error.js" as="script"/>
      <link rel="preload" href="/_next/static/runtime/webpack.js" as="script"/>
      <link rel="preload" href="/_next/static/runtime/main.js" as="script"/>
   </head>
   <body>
      <div id="__next"></div>
      <script src="/_next/static/development/dll/dll_4a2ab6ce0cb456fbfead.js"></script><script>__NEXT_DATA__ = {"props":{"pageProps":{}},"page":"/","pathname":"/","query":{},"buildId":"development"};__NEXT_LOADED_PAGES__=[];__NEXT_REGISTER_PAGE=function(r,f){__NEXT_LOADED_PAGES__.push([r, f])}</script><script async="" id="__NEXT_PAGE__/" src="/_next/static/development/pages/index.js"></script><script async="" id="__NEXT_PAGE__/_app" src="/_next/static/development/pages/_app.js"></script><script async="" id="__NEXT_PAGE__/_error" src="/_next/static/development/pages/_error.js"></script><script src="/_next/static/runtime/webpack.js" async=""></script><script src="/_next/static/runtime/main.js" async=""></script>
   </body>
</html>
```

After: 
```html
<!DOCTYPE html>
<html>
   <head>
      <meta charSet="utf-8" class="next-head"/>
      <link rel="preload" href="/_next/static/development/pages/index.js" as="script"/>
      <link rel="preload" href="/_next/static/development/pages/_app.js" as="script"/>
      <link rel="preload" href="/_next/static/development/pages/_error.js" as="script"/>
      <link rel="preload" href="/_next/static/runtime/webpack.js" as="script"/>
      <link rel="preload" href="/_next/static/runtime/main.js" as="script"/>
   </head>
   <body>
      <div id="__next">
         <h1>This example integrates react-i18next for simple internationalization.</h1>
         <div>
            <h1>welcome to next.js</h1>
            <p>This example integrates react-i18next for simple internationalization.</p>
            <p>test words for en</p>
            <div><button>fire in the wind for en</button></div>
            <p>You can either pass t function to child components.</p>
            <p>Or wrap your component using the translate hoc provided by react-i18next.</p>
            <p>Alternatively, you can use <code>Trans</code> component.</p>
            <a href="/page2">Go to page 2</a><br/><a href="/page3">Go to page 3 (no hoc)</a>
         </div>
      </div>
      <script src="/_next/static/development/dll/dll_4a2ab6ce0cb456fbfead.js"></script><script>__NEXT_DATA__ = {"props":{"pageProps":{"i18n":null,"initialI18nStore":{"en":{"home":{"welcome":"welcome to next.js","sample_test":"test words for en","sample_button":"fire in the wind for en","link":{"gotoPage2":"Go to page 2","gotoPage3":"Go to page 3 (no hoc)"}},"common":{"integrates_react-i18next":"This example integrates react-i18next for simple internationalization.","pureComponent":"You can either pass t function to child components.","extendedComponent":"Or wrap your component using the translate hoc provided by react-i18next.","transComponent":"Alternatively, you can use \u003c1\u003eTrans\u003c/1\u003e component."}}},"initialLanguage":"en-US"}},"page":"/","pathname":"/","query":{},"buildId":"development"};__NEXT_LOADED_PAGES__=[];__NEXT_REGISTER_PAGE=function(r,f){__NEXT_LOADED_PAGES__.push([r, f])}</script><script async="" id="__NEXT_PAGE__/" src="/_next/static/development/pages/index.js"></script><script async="" id="__NEXT_PAGE__/_app" src="/_next/static/development/pages/_app.js"></script><script async="" id="__NEXT_PAGE__/_error" src="/_next/static/development/pages/_error.js"></script><script src="/_next/static/runtime/webpack.js" async=""></script><script src="/_next/static/runtime/main.js" async=""></script>
   </body>
</html>
```
@timneutkens timneutkens added help wanted good first issue Easy to fix issues, good for newcomers labels Nov 12, 2018
@timneutkens timneutkens added Type: Bug and removed good first issue Easy to fix issues, good for newcomers help wanted labels Mar 9, 2019
timneutkens pushed a commit that referenced this issue Mar 14, 2019
This will allow symlinked assets to be removed from the cache as well

Fixes: #5620
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants