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

Output File Tracing not working for ISR page #34246

Closed
Mitsunee opened this issue Feb 11, 2022 · 13 comments · Fixed by #34269
Closed

Output File Tracing not working for ISR page #34246

Mitsunee opened this issue Feb 11, 2022 · 13 comments · Fixed by #34269
Labels
Output (export/standalone) Related to the the output option in `next.config.js`.

Comments

@Mitsunee
Copy link

Mitsunee commented Feb 11, 2022

Run next info (available from version 12.0.8 and up)

Operating System:
  Platform: linux
  Arch: x64
  Version: #31-Ubuntu SMP Thu Jan 13 17:41:06 UTC 2022
Binaries:
  Node: 14.18.2
  npm: 8.5.0
  Yarn: 1.22.17
  pnpm: 6.30.1
Relevant packages:
  next: 12.0.9
  react: 17.0.2
  react-dom: 17.0.2

What version of Next.js are you using?

12.0.9

What version of Node.js are you using?

14.18.2

What browser are you using?

chromium-based

What operating system are you using?

Kubuntu

How are you deploying your application?

vercel

Describe the Bug

I am currently (with the help of Vercel Support, who also advised me to open an Issue here) debugging why ISR is not working for my project and we've narrowed down the issue to Output File Tracing not catching any of the filesystem calls in my getStaticProps function as they are wrapped in convenience functions specifically written for next.js (in the @foxkit/node-util module that I wrote for this purpose). Even after specifically including unstable_includeFiles: ["assets/data", "public/assets/backgrounds"] in the next.config.mjs file none of the calls are recognized and the directory available during ISR revalidation remains basically empty other than the .next directory (which btw does not contain a directory called cache anymore).

I have also made a little test app that scans the working directory which somehow has the same issue despite not using the @foxkit/node-util package: https://nextjs-playground-mitsunee.vercel.app/
This app imports fs from "fs/promises" and uses the readdir and stat methods, neither of which appear to get recognized by Output File Tracing.

Expected Behavior

for fgo-tools: The entire directories assets/data and public/assets/backgrounds are available in the working directory of all serverless functions as per configuration file.

for the test app: The entire directory excluding node_modules was touched using fs.promises.stat and should thus be available.

To Reproduce

I am honestly not entirely sure. I have a PR on my fgo-tools repo with the issue as well as the test app's repo

@Mitsunee Mitsunee added the bug Issue was opened via the bug report template. label Feb 11, 2022
@balazsorban44 balazsorban44 added the Output (export/standalone) Related to the the output option in `next.config.js`. label Feb 11, 2022
@balazsorban44
Copy link
Member

balazsorban44 commented Feb 11, 2022

I found one reference which might need the manual unstable_includeFiles configuration since the path is defined dynamically:

https://github.com/Mitsunee/fgo-tools/blob/b5886ca7d89e9f052e2bc0164aaf96982cf241a1/pages/events/%5Bslug%5D.jsx#L176

From the docs:

In those cases, you can export page configs props unstable_includeFiles and unstable_excludeFiles respectively. Each prop accepts an array of globs relative to the project's root to either include or exclude in the trace.

So just to make sure, could you test out /assets/data/events/** as a glob pattern? I don't think assets/data in itself will match those files you are referencing.

If that works, I still advise being as specific as possible about the inclusions to reduce the output size.

Let me know if that works!

tested the above pattern on https://globster.xyz/ with the following data:

/assets/data/events/0.yml
/assets/data/events/1.yml
/assets/data/events/2.yml
/assets/data/events/3.yml
/assets/data/login-tickets/4.yml
/assets/data/login-tickets/5.yml
/assets/data/upgrades/6.yml
/assets/data/upgrades/7.yml
/assets/8.svg
/assets/svg/9.svg

UPDATE

Correction on the above, I re-read our docs, and I believe we can make it slightly easier to understand (I highlighted where it says page configs), but the unstable_includeFiles is a page config, not a next.config.js option, which means you will need to do the following:

In pages/events/[slug].js, add the following:

export const config = {
  unstable_includeFiles: ["assets/data/events/*.yml"]
};

As you can see below, this will include the desired assets in the standalone app:

image

As a caution, I would like to note that any static assets added after the server has started won't be available.

https://nextjs.org/docs/basic-features/static-file-serving

@balazsorban44 balazsorban44 added area: documentation and removed bug Issue was opened via the bug report template. labels Feb 11, 2022
@Mitsunee
Copy link
Author

The page in the fgo-tools repo that is using ISR is the index.jsx page, not events/[slug].jsx.
Would exporting the config from that page and adding double globstars (**) be enough to catch all the files? I am trying to keep the data in that directory as relevant as possible so a simple "match anything in that dir" should be exactly what the expected result of tracing would be in an ideal scenario

@balazsorban44
Copy link
Member

Hmm. Looking into https://github.com/Mitsunee/fgo-tools/blob/main/pages/index.jsx I cannot see a getStaticProps. Could you elaborate?

@Mitsunee
Copy link
Author

It's hiding in line 3:

https://github.com/Mitsunee/fgo-tools/blob/b5886ca7d89e9f052e2bc0164aaf96982cf241a1/pages/index.jsx#L3

It is exporting the function imported from this file: https://github.com/Mitsunee/fgo-tools/blob/main/src/server/HomePage/index.js

I did not like having serverside code bunched together with frontend code so I decided to split of SSG/SSR/ISR into its own directory

@balazsorban44
Copy link
Member

balazsorban44 commented Feb 11, 2022

"match anything in that dir" should be exactly what the expected result of tracing would be in an ideal scenario

The ideal scenario is to keep the bundle size down IMO, but that is of course your design decision. I think the following should be enough then: **/*.yml

You will need to give a list of files, not directories, so ** in itself won't work.

@balazsorban44
Copy link
Member

balazsorban44 commented Feb 11, 2022

Thinking about it, it probably does not really matter which page you export this from, as the standalone server will serve them all the same. I believe we just merge the different pages configs for this at build-time. It's more about colocation, keeping your config together with the pages that need those files.

@Mitsunee
Copy link
Author

The ideal scenario is to keep the bundle size down IMO, but that is of course your design decision. I think the following should be enough then: **/*.yml

You will need to give a list of files, not directories, so ** in itself won't work.

Matching all the files is actually as small as bundle could possibly be without missing data (which is causing the errors I currently have). I will try assets/data/**/*.(json|yml) tomorrow (need sleep first, it's almost midnight here) and think about a way to maybe improve bundle size at a later time.

@balazsorban44
Copy link
Member

Sounds good, let me know if that helps! I cloned your repo and with the config added, you can see #34246 (comment) that assets are copied over 👍

@Mitsunee
Copy link
Author

Mitsunee commented Feb 12, 2022

I tried the following in my playground repository which did not seem to work:

https://github.com/Mitsunee/nextjs-playground/blob/4a2e35230f60ee6ae863e9dd1f37d10ec969bfdf/pages/index.js#L10-L12

// index.js
export const config = {
  unstable_includeFiles: ["public/**/*.(ico|svg)"]
};

(Deployed here: nextjs-playground-b5cquk25t-mitsunee.vercel.app)

For reference the public directory contains the svg icons seen in the ui of that deployment as well as the default favicon from vercel. I have not yet received any logs on logtail through the integration


Edit: In the fgo-tools project I was also trying assets/**/*.yml, but I either missed the first run or it failed to export logs to logtail. As ISR is set to an hour there I guess I'll just wait if logs appear later and take care of some other stuff until then.

Edit2: It has been over an hour now and I've neither gotten any logs on logtail nor do I see the result of ISR running for the configure-output-file-tracing branch (build time would be an obvious thing that should change as it's directly returning Date.now() in getStaticProps. Not sure what the problem there is to be honest.
See https://github.com/Mitsunee/fgo-tools/blob/configure-output-file-tracing/src/server/HomePage/index.js and https://github.com/Mitsunee/fgo-tools/blob/configure-output-file-tracing/pages/index.jsx for the code.

@ijjk
Copy link
Member

ijjk commented Feb 12, 2022

Hi @Mitsunee so the above includeFiles glob isn't working since it's not a regex but a minimatch glob instead which would be good for us to ensure is documented.

Updating the glob to this should resolve the issue:

export const config = {
  unstable_includeFiles: ["public/**/*.*(svg|ico)"]
}

PR updating documentation for this opened here #34269

@kodiakhq kodiakhq bot closed this as completed in #34269 Feb 12, 2022
kodiakhq bot pushed a commit that referenced this issue Feb 12, 2022
This ensures we reference [minimatch](https://www.npmjs.com/package/minimatch) since the globs there can have different syntax. 

## Documentation / Examples

- [x] Make sure the linting passes by running `yarn lint`

Closes: #34246
@Mitsunee
Copy link
Author

Mitsunee commented Feb 12, 2022

I've confirmed that adjusting the pattern fixes the playground app, but I am still unsure about why these patterns did not work for my actual app:

https://github.com/Mitsunee/fgo-tools/blob/configure-output-file-tracing/pages/index.jsx

export const config = {
  unstable_includeFiles: [
    "assets/data/**/*.yml",
    "public/assets/backgrounds/landing/*.png"
  ]
};

I used pthrasher's minimatch tester to confirm that the patterns should match:

Screenshot_20220212_185629

The intial deployment (SSG) ran without problems and I can also run everything just fine locally, so I'm assuming ISR is still throwing due to the files not being there

@ijjk
Copy link
Member

ijjk commented Feb 12, 2022

@Mitsunee it looks like you're only including .png files but the directory contains .jpg files https://github.com/Mitsunee/fgo-tools/tree/main/public/assets/backgrounds/landing

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Output (export/standalone) Related to the the output option in `next.config.js`.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants