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

Treat fs-extra as an alias to fs / node:fs #293

Closed
kachkaev opened this issue Jun 9, 2022 · 4 comments · Fixed by #296
Closed

Treat fs-extra as an alias to fs / node:fs #293

kachkaev opened this issue Jun 9, 2022 · 4 comments · Fixed by #296

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Jun 9, 2022

I switched from plain fs to fs-extra in a project that uses Next.js and Vercel. After doing so I noticed that some Vercel lambdas ‘lost’ local files, which was really strange. The only solution I found was switching back to const fs = require("fs").

About four months later I finally managed to discover this repo as well as *.js.nft.json files inside .next/server/pages. It all makes sense now 😅

Given the recent addition node:fs in #285, would it be feasible to support fs-extra too? I understand that supporting every third-party lib is not worth it, but fs-extra is pretty big:

Screenshot 2022-06-09 at 21 33 45

The mystery of ‘lost’ files in lambdas was bugging me for quite some time and I spent about two working days on figuring things out. If fs-extra was supported out of box before I switched to it, I’d be much less puzzled about Vercel! 🙂

What do you think? Happy to help with a PR or with testing!

@styfle
Copy link
Member

styfle commented Jun 10, 2022

Generally, this is already handled by const file = path.join(process.cwd(), 'file.txt') which would be analyzed prior to the actual fs call const content = await fs.readFile(file).

Will path.join() work for your use case?

@kachkaev
Copy link
Contributor Author

kachkaev commented Jun 10, 2022

👋 @styfle! Interesting! Is there any way to use path with globs? E.g. here:

glob
  .sync(`${process.cwd()}/public/blocks/**/block-metadata.json`)
  .map((path: string): ExpandedBlockMetadata => { fs.readFileSync(path); })

UPD: Using path.resolve and path.join calls seem to do the job, even if I swap fs with fs-extra! 🎉 I just ran an experiment in blockprotocol/blockprotocol#339 to confirm. Not sure if glob or glob.sync are analysed, but it does not seem to affect the results in my case.

It’d be still nice to support fs-extra because the trap our team has fallen into may affect someone else too. What @vercel/nft does behind the scenes is pure magic until you discover *.nft.json files in .next. It may take a while! 😅

@styfle
Copy link
Member

styfle commented Jun 10, 2022

It would be helpful if you can share a case that works with fs, but not fs-extra

@kachkaev
Copy link
Contributor Author

kachkaev commented Jun 11, 2022

I created a new test case in #296. I wanted to work on the issue locally, but could not do yarn install and gave up 😅:

error /private/tmp/nft/node_modules/oniguruma: Command failed.
Exit code: 1
Command: node-gyp rebuild
Arguments: 
Directory: /private/tmp/nft/node_modules/oniguruma
Output:
gyp info it worked if it ends with ok
gyp info using node-gyp@5.1.0
gyp info using node@14.19.3 | darwin | x64
gyp info find Python using Python version 3.9.13 found at "/opt/homebrew/opt/python@3.9/bin/python3.9"
gyp info spawn /opt/homebrew/opt/python@3.9/bin/python3.9
gyp info spawn args [
gyp info spawn args   '/Users/ak/.nvm/versions/node/v14.19.3/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',

Might be related to my M1 mac and ranisalt/node-argon2#305. I tried Node 14, 16 and 18, but no luck so far. Happy to dig into this yarn install problem further if you need my help!

styfle added a commit that referenced this issue Jul 13, 2022
- Fixes #293

Co-authored-by: Steven <steven@ceriously.com>
devexpert98 added a commit to devexpert98/nft that referenced this issue Aug 16, 2022
- Fixes vercel/nft#293

Co-authored-by: Steven <steven@ceriously.com>
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

Successfully merging a pull request may close this issue.

2 participants