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

[Bug] fs patch doesn't handle { bigint: true } #2232

Closed
1 task done
andreialecu opened this issue Dec 11, 2020 · 19 comments · Fixed by #2262
Closed
1 task done

[Bug] fs patch doesn't handle { bigint: true } #2232

andreialecu opened this issue Dec 11, 2020 · 19 comments · Fixed by #2262
Assignees
Labels
bug Something isn't working upholded Real issues without formal reproduction

Comments

@andreialecu
Copy link
Contributor

  • I'd be willing to implement a fix

Describe the bug

When running on Node 15.4.0, the following error can occur:

node:fs:1931
      const dev = BigIntPrototypeToString(stats.dev, 32);
                  ^

TypeError: BigInt.prototype.toString requires that 'this' be a BigInt
    at Number.toString (<anonymous>)
    at gotStat (node:fs:1931:19)

This started happening because of this commit: https://github.com/nodejs/node/blob/8d6c2f2ada79e52ec0b376769a7d94814945bd4f/lib/fs.js#L1931

The root cause is that the fake fs implementation doesn't correctly handle the { bigint: true } option that the various fs.stat functions can take:

https://nodejs.org/api/fs.html#fs_class_fs_stats

To Reproduce

$ nvm install 15.4.0
$ yarn init -2

update package.json to:
{
  "name": "bug",
  "scripts": {
    "start": "echo test",
    "bug": "yarn bug"
  }
}

$ yarn
$ yarn bug

➜ yarn bug
node:fs:1931
      const dev = BigIntPrototypeToString(stats.dev, 32);
                  ^

TypeError: BigInt.prototype.toString requires that 'this' be a BigInt
    at Number.toString (<anonymous>)
    at gotStat (node:fs:1931:19)
    at /private/var/folders/j7/mjhkhz7x15z0nmt42b5k2x980000gn/T/tmp.K9NgZraS/.pnp.js:4516:13

Screenshots

If applicable, add screenshots to help explain your problem.

Environment if relevant (please complete the following information):

  • OS: OSX
  • Node version 15.4.0
  • Yarn version 2.4.0

Additional context

https://discord.com/channels/226791405589233664/226793713722982400/786920513238073395

@andreialecu andreialecu added the bug Something isn't working label Dec 11, 2020
@merceyz merceyz added the upholded Real issues without formal reproduction label Dec 11, 2020
@daryapotanina
Copy link

daryapotanina commented Dec 12, 2020

Same problem with yarn 2.4.0 (zero install) and ts-node-dev.

$ cat ./file-system.ts
console.log("123");

~/projects/ts-data-analysis-with-zero-install/examples on  main! ⌚ 1:26:29
$ yarn ts-node ./file-system.ts
123

~/projects/ts-data-analysis-with-zero-install/examples on  main! ⌚ 1:26:53
$ yarn ts-node-dev ./file-system.ts
[INFO] 01:27:04 ts-node-dev ver. 1.1.1 (using ts-node ver. 9.1.1, typescript ver. 4.1.3)
TypeError: BigInt.prototype.toString requires that 'this' be a BigInt
    at Number.toString (<anonymous>)
    at gotStat (node:fs:1931:19)
    at /Users/darya/projects/ts-data-analysis-with-zero-install/.pnp.js:7049:13

Environment:

$ node -v && npm -v
v15.4.0
7.0.15

OS: OSX

Code: https://github.com/avevlad/ts-data-analysis-with-zero-install/tree/39223a836097ded6201592364b925798c0fddea5

@xandris
Copy link

xandris commented Dec 14, 2020

I started making a branch that implements that option in the FakeFS interfaces and up through all the implementers, but if y'all are working on it already you can probably do it a lot faster than I can as an outsider.

@xandris
Copy link

xandris commented Dec 15, 2020

I have a pull request open for @types/node to fix definitions for stat/lstat/fstat in callback, synchronous, promisify, and promises mode. I don't think you can successfully write a shim for fs if the definitions aren't right, and I don't think the definitions are right in the @types/node package. (For example, they have an fstat in fs.promises, and that doesn't appear to exist.)

@franciscofabian
Copy link

I have the same issue with yarn 2.4.0 and node 15.4.0, but not with previous versions of node

@merceyz merceyz self-assigned this Dec 22, 2020
@andreialecu
Copy link
Contributor Author

andreialecu commented Dec 29, 2020

I can confirm that #2262 fixes it. It can be used via:

yarn set version from sources --branch 2262

Make sure to run yarn install afterwards.

MartinRosenberg added a commit to MartinRosenberg/typescript-exercises that referenced this issue Feb 1, 2021
Yarn 2 has this bug with Node 15. The bug is fixed in a branch that
hasn't been published yet. The choice was between `yarn set version
from sources --branch 2262` and then remember to switch back to main
at some later date, or just use Node LTS. I opted for stability.

yarnpkg/berry#2232
@stavalfi
Copy link

@arcanis I'm facing the following issue with node 15.10.0, but not in 14.16.0:

    TypeError: BigInt.prototype.toString requires that 'this' be a BigInt
        at Number.toString (<anonymous>)

      17650 |         process.nextTick(() => {
      17651 |           fakeImpl.apply(fakeFs, args).then(result => {
    > 17652 |             callback(null, result);
            |             ^
      17653 |           }, error => {
      17654 |             callback(error);
      17655 |           });

      at .pnp.js:17652:13

Is this related to this issue? (yarn -v === 2.4.0)

@merceyz
Copy link
Member

merceyz commented Feb 26, 2021

Yes, see #2232 (comment)

@niieani
Copy link
Contributor

niieani commented Mar 20, 2021

@arcanis Odd, I'm still getting this issue with the latest yarn (2.4.1), even though it's been released only few weeks ago, and the PR was merged in December. Maybe the relevant packages weren't upgraded in the release?

@arcanis
Copy link
Member

arcanis commented Mar 20, 2021

The 2.4.1 only includes a TS compat fix. It'll be part of the 3.0, which takes a bit of time due to me being on vacation at the moment - will release when I'm back.

@ericvera
Copy link

Is there by any chance an up-to-date branch? It seems this one is missing other critical updates that were already in 2.4.1. Seems related to a Typescript patch I saw referenced in another issue.

I can confirm that #2262 fixes it. It can be used via:

yarn set version from sources --branch 2262

Make sure to run yarn install afterwards.

Output of yarn install after yarn set version from sources --branch 2262.

eric@Erics-MacBook-Air functions % yarn install
➤ YN0000: ┌ Resolution step
➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code
➤ YN0000: └ Completed in 0s 318ms
➤ YN0000: ┌ Fetch step
➤ YN0013: │ typescript@patch:typescript@npm%3A4.1.5#builtin<compat/typescript>:
➤ YN0013: │ typescript@patch:typescript@npm%3A4.2.3#builtin<compat/typescript>:
➤ YN0066: │ typescript@patch:typescript@npm%3A4.2.3#builtin<compat/typescript>::version=4.2.3&hash=cc6730: Cannot apply hunk #6 (set enableInlineHunks for details)
➤ YN0000: └ Completed in 1s 61ms
➤ YN0000: Failed with errors in 1s 384ms

@ericvera
Copy link

The 2.4.1 only includes a TS compat fix. It'll be part of the 3.0, which takes a bit of time due to me being on vacation at the moment - will release when I'm back.

Curious... is there a scale to a bit of time? :) days, weeks, months?
(trying to assess if reverting all the work to get PnP working in my big project is worth it to make one of the workspaces work or if I can wait and keep all the PnP goodness I've been enjoying on the other workspaces)

@merceyz
Copy link
Member

merceyz commented Mar 23, 2021

Is there by any chance an up-to-date branch?

Since it has been merged master would be the most up to date

yarn set version from sources

@ericvera
Copy link

That fixed the issue. Thanks!

@NicMul
Copy link

NicMul commented Mar 25, 2021

Is there by any chance an up-to-date branch?

Since it has been merged master would be the most up to date

yarn set version from sources

This worked! Just remember to yarn install afterwards.

@mfrederiksen
Copy link

Any chance of this getting ported into the 2.4 branch or will it only ship in 3.0?

@merceyz
Copy link
Member

merceyz commented Apr 13, 2021

It has been released in 3.0.0-rc.1 and 3.0.0-rc.2, no plans to backport it

JLHwung added a commit to babel/babel-loader that referenced this issue Apr 20, 2021
@bryanvaz

This comment has been minimized.

@develar
Copy link

develar commented Apr 27, 2021

This comment was marked as disruptive content.

But actually nothing wrong in this comment. I tired of various bugs and Python-like situation (1 or 2, ), missing IDE support for PNP and switched to PNPM (strict resolve as Yarn 2 does without broken IDE support and without introducing non-standard elements to workflow (no need to put yarn 2 under project, no need to use special CLI to install plugins and so on)).

@arcanis
Copy link
Member

arcanis commented Apr 27, 2021

But actually nothing wrong in this comment.

Nothing good either, which is just as annoying. An issue tracker isn't the right device to vent your frustrations. Be productive, or don't be here.

Also note that it literally takes one command to use the RELEASE CANDIDATE version, which includes the fix. If you don't wish to use it fine, but have the elegance to skip posting meme gifs on this bug tracker.

@yarnpkg yarnpkg locked as resolved and limited conversation to collaborators Apr 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working upholded Real issues without formal reproduction
Projects
None yet
Development

Successfully merging a pull request may close this issue.