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

Different behavior for same code: Single app & Monorepo #344

Open
intpp opened this issue Feb 9, 2023 · 3 comments
Open

Different behavior for same code: Single app & Monorepo #344

intpp opened this issue Feb 9, 2023 · 3 comments

Comments

@intpp
Copy link

intpp commented Feb 9, 2023

I'm working with monorepo of Next.js apps + nx and found some strange behavior, the file next-i18next.config.js (from the next-i18next package), which is used by an application, doesn't apply to the build.

So I did some research and found that behavior for the package @vercel/nft is different when we run a script with the package to analyze files. I have written a small package with the same idea of files loading as the next-i18next package to show the issue and a few examples which are using the package.

Script

const path = require('path');
const { nodeFileTrace } = require('@vercel/nft');

(async () => {
  const files = [path.resolve(__dirname, 'app.js')];

  const { fileList } = await nodeFileTrace(files);

  console.log('fileList', fileList);
})();

Single application/package project

npm run files

> vercel-nft-demo@1.0.0 files
> node nft.js

fileList Set(6) {
  'app.js',
  'package.json',
  'node_modules/intpp-some-nft-demo/package.json',
  'node_modules/intpp-some-nft-demo/index.js',
  'config.js',
  'assets/common.json'
}

Monorepo from a project's root

npm run files:app1

> vercel-nft-workspace-demo@1.0.0 files:app1
> node apps/app1/nft.js

fileList Set(4) {
  'apps/app1/app.js',
  'apps/app1/package.json',
  'node_modules/intpp-some-nft-demo/package.json',
  'node_modules/intpp-some-nft-demo/index.js'
}

Monorepo from an application's root

npm run files

> vercel-nft-demo@1.0.0 files
> node nft.js

fileList Set(2) { 'app.js', 'package.json' }

Path for the file app.js the same.

Demo repos:
https://github.com/intpp/vercel-nft-demo
https://github.com/intpp/vercel-nft-workspace-demo

@belong-bryce
Copy link

Isn't that because of the location-dependent behaviours?

From the readme:

Process Cwd
When applying analysis certain functions rely on the process.cwd() value, such as path.resolve('./relative') or even a direct process.cwd() invocation.

Setting the processCwd option allows this analysis to be guided to the right path to ensure that assets are correctly detected.

const { fileList } = await nodeFileTrace(files, {
  processCwd: path.resolve(__dirname)
}
By default processCwd is the same as base.

@intpp
Copy link
Author

intpp commented Feb 21, 2023

Probably yes, but I think it shouldn't affect detecting correct files because the same target file still has the same dependencies. Am I wrong?

I have updated monorepo's examples with the options base and processCwd.

Next.js has "experimental.outputFileTracingRoot" and it still doesn't work perfectly for monorepos because of this issue...

@belong-bryce
Copy link

it shouldn't affect detecting correct files because the same target file still has the same dependencies. Am I wrong?

I think you're right, but this line seems to imply that the files it detects might be dependent on the cwd setting:

Setting the processCwd option allows this analysis to be guided to the right path to ensure that assets are correctly detected.

If that's the case, then we probably need to find the magical combination of callsite + cwd settings to give expected behaviour in any scenario.

I'm really interested in seeing you succeed with this challenge for my own use-cases. Does the update you made to your examples give any different results to your initial reports?

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

No branches or pull requests

2 participants