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

resolve v1.21 breaks bundled graceful-fs in nextjs #257

Closed
benbender opened this issue Jan 5, 2022 · 8 comments · Fixed by #258
Closed

resolve v1.21 breaks bundled graceful-fs in nextjs #257

benbender opened this issue Jan 5, 2022 · 8 comments · Fixed by #258

Comments

@benbender
Copy link

benbender commented Jan 5, 2022

As debugged in browserify/resolve#264, resolve v1.21 introduces a change in behaviour which in turn breaks graceful-fs. A fix in graceful-fs is released with v4.2.9, but nft seems to be bundling a lower version.

As this potentially breaks a lot of next.js babel-setups (see f.e. vercel/next.js#33003), a rerelease with the fixed graceful-fs-package would be great!

@ljharb
Copy link

ljharb commented Jan 5, 2022

It would also be ideal if this package didn't bundle requireable things at all, so that downstream consumers benefited from automatic in-range version updates for bugfixes (like this one) and security fixes.

@styfle
Copy link
Member

styfle commented Jan 5, 2022

Can you share the steps to reproduce the issue?

@benbender
Copy link
Author

"You could use https://github.com/vercel/next.js/tree/canary/examples/with-babel-macros as a quite simple reproduction for testing. Simply clone it and run "npm install && npm dev" to see the error in the browser."

as written here browserify/resolve#264 (comment)

@ljharb
Copy link

ljharb commented Jan 5, 2022

Note that if you update graceful-fs and recreate the bundle, that should be sufficient to solve the short-term problem.

@styfle
Copy link
Member

styfle commented Jan 5, 2022

So this isn't an issue with nft which doesn't rely on resolve?

@ljharb
Copy link

ljharb commented Jan 5, 2022

ahhh right, this was filed on the wrong repo (about relying on graceful-fs, not on resolve). It's an issue with the index.js file in https://unpkg.com/browse/next@12.0.7/dist/compiled/@vercel/nft/ which bundles the broken graceful-fs instead of requiring it.

This should be closed and filed instead of the repo for the next package.

@styfle styfle closed this as completed Jan 5, 2022
@styfle
Copy link
Member

styfle commented Jan 5, 2022

Upon further investigation, I think the issue is the global patch here:

gracefulify(fs);

I think we can change nft to call the read/write methods directly instead of doing a global patch like this.

@styfle styfle reopened this Jan 5, 2022
@ljharb
Copy link

ljharb commented Jan 5, 2022

That would also be a great long-term solution for nft :-)

(altho it'd still require next to update its bundle and/or stop bundling third party deps)

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.

3 participants