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

for loop, inside async setter doesn't wait all Promise. #3898

Closed
duecorda opened this issue Jun 19, 2022 · 2 comments
Closed

for loop, inside async setter doesn't wait all Promise. #3898

duecorda opened this issue Jun 19, 2022 · 2 comments

Comments

@duecorda
Copy link

duecorda commented Jun 19, 2022

Version

v16.15.1

Platform

Linux ubuntu 5.13.0-51-generic nodejs/node#58~20.04.1-Ubuntu SMP Tue Jun 14 11:29:12 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

It should return { q: 'q', w: 'w', e: 'e', r: 'r', a: 'a'}
But { q: 'q', w: 'w' } or { q: 'q', w: 'w', e: 'e' }, depends on your system.

class Foo {
  static async build(attrs={}) {
    const _instance = new this();
    await (_instance.attributes = attrs);
    return _instance;
  }

  constructor() {
    this.props = {};

    Object.defineProperty(this, 'attributes', {
      get() {
        return this.props;
      },
      async set(attrs) {
        for (const k in attrs) {
          await (this.props[k] = attrs[k]);
        };
      }
    });
  }
}

async function main() {
  const attrs = { q: 'q', w: 'w', e: 'e', r: 'r', a: 'a'};
  const foo = await Foo.build(attrs);
  console.log(foo.attributes); // incomplete.
};

main();

for...of returns same result.

for (const kv of Object.entries(attrs)) {
  await (this.props[kv[0]] = kv[1]);
}

But it works Object.keys...forEach

Object.keys(attrs).forEach(async (k) => {
  await (this.props[k] = attrs[k]);
});

and works too Object.entries...map

Object.entries(attrs).map(async (kv) => {
  await (this.props[kv[0]] = kv[1]);
});

How often does it reproduce? Is there a required condition?

Everytime.

What is the expected behavior?

{ q: 'q', w: 'w', e: 'e', r: 'r', a: 'a' }

What do you see instead?

{ q: 'q', w: 'w', e: 'e' }

Additional information

As I wrote, I can loop with Object.keys.
but I've spent several hours to find this problem.
If I'm not wrong with that code - using for..in loop inside async setter - hope it fixed.

@aduh95
Copy link

aduh95 commented Jun 19, 2022

The promise your setter returns in ignored by the engine, it will treat it as a truthy boolean value and that's all. So when you write await (_instance.attributes = attrs);, _instance.attributes = attrs will evaluate to the value of attrs, not the Promise your setter returns.

If you add a setTimeout(() => console.log(foo.attributes), 300);, you'll see that all the properties are there eventually.

Trying to set property asynchronously is dangerous, you can get into race conditions very easily if more than one async branch tries to write data concurrently, I wouldn't recommend it.

If async setters is really what you want, here are some (not so good) examples

You'd either need to expose some property that returns a promise that resolves when the instance is ready:

class Foo {
  static async build(attrs = {}) {
    const _instance = new this();
    _instance.attributes = attrs;
    await _instance.areAttributesReady;
    return _instance;
  }

  #props = {};
  #propsPromise = Promise.resolve();

  get areAttributesReady() {
    return this.#propsPromise;
  }

  set attributes(attrs) {
    this.#propsPromise = (async (previousPromise) => {
      await previousPromise;
      await Promise.all(Object.entries(attrs).map(async ([key, value]) => {
        this.#props[key] = await value;
      }));
      this.#propsPromise = Promise.resolve();
    })(this.#propsPromise);
  }

  get attributes() {
    return this.#props;
  }
}

async function main() {
  const attrs = { q: 'q', w: 'w', e: 'e', r: 'r', a: 'a' };
  const foo = await Foo.build(attrs);
  console.log(foo.attributes); // incomplete.
}

main().catch(console.log);

Or make an async method:

class Foo {
  static async build(attrs = {}) {
    const _instance = new this();
    await _instance.setAttributes(attrs);
    return _instance;
  }

  #props = {};

  async setAttributes(attrs) {
      await Promise.all(Object.entries(attrs).map(async ([key, value]) => {
        this.#props[key] = await value;
      }));
  }

  get attributes() {
    return this.#props;
  }
}

async function main() {
  const attrs = { q: 'q', w: 'w', e: 'e', r: 'r', a: 'a' };
  const foo = await Foo.build(attrs);
  console.log(foo.attributes); // incomplete.
}

main().catch(console.log);

@aduh95 aduh95 transferred this issue from nodejs/node Jun 19, 2022
@duecorda
Copy link
Author

Thank you for your kind answer.
May I need change my structure.

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