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

outputJson does not preserve the object for asynchronous processing #702

Closed
paragi opened this issue Jun 30, 2019 · 4 comments · Fixed by #768
Closed

outputJson does not preserve the object for asynchronous processing #702

paragi opened this issue Jun 30, 2019 · 4 comments · Fixed by #768

Comments

@paragi
Copy link

paragi commented Jun 30, 2019

  • Operating System: Linux 4.15.0-52 Debian
  • Node.js version: 12.2.0
  • fs-extra version:8.1.0

When writing object asynchronously with outputJson, the object is not preserved, before the promise is returned.

Example:

const fs = require('fs-extra');

( async () => {
  let i;
  let obj = {}
  let promises = [];

  for( i = 0; i<4; i++ ){
    obj.id = i;
    promises.push( fs.outputJson('./' + i, obj) );
  }

  await Promise.all(promises);

  for( i = 0; i<4; i++ )
    console.log( await fs.readJson('./' + i));

})();
@RyanZim
Copy link
Collaborator

RyanZim commented Jul 1, 2019

Yes, this is normal. If you're mutating an object, behavior will not be predictable.

@RyanZim RyanZim closed this as completed Jul 1, 2019
@paragi
Copy link
Author

paragi commented Jul 1, 2019

In most asynchronous use cases i can imagine, you would want to use the stringification, to preserve the object, instead of copying it first (closure), then stringify it and then dumping it.

In my opinion, it seems contra intuitive, and sort of defeats the purpose, of having a function to replace fs.outputFile(fileName, JSON.stringify(object) )

I wish you would reconsider.

@RyanZim
Copy link
Collaborator

RyanZim commented Jul 1, 2019

How does writeJson work here?

@RyanZim RyanZim reopened this Jul 1, 2019
@paragi
Copy link
Author

paragi commented Jul 1, 2019

writeJson seems to be asynchronous mutation safe

@RyanZim RyanZim added this to the 9.0.0 milestone Jan 30, 2020
RyanZim added a commit that referenced this issue Mar 10, 2020
@RyanZim RyanZim mentioned this issue Mar 10, 2020
RyanZim added a commit that referenced this issue Mar 11, 2020
* Upgrade jsonfile

Fixes #745

* Test from main entry point

* Make outputJson() mutation-proof

Fixes #702

* Update outputJsonSync() to match async implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants