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

timeoutId does not implement unref() method #6735

Open
gajus opened this issue Aug 13, 2018 · 8 comments
Open

timeoutId does not implement unref() method #6735

gajus opened this issue Aug 13, 2018 · 8 comments
Labels
Library definitions Issues or pull requests about core library definitions

Comments

@gajus
Copy link

gajus commented Aug 13, 2018

/* @flow */

const foo: TimeoutID = setTimeout(() => {}, 300);

foo.unref();
5: foo.unref();
       ^ Cannot call `foo.unref` because property `unref` is missing in `TimeoutID` [1].
References:
3: const foo: TimeoutID = setTimeout(() => {}, 300);
              ^ [1]

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBjOA7AzgFzCjjgC4wAVASwFsBTOAV3wEkARMAXjFzv2vpN8ACmEBKLgD4wAbwC+AGjABmAAyqxAbnTE4AOkbYATnSjjNQA

Refer to:

@mgtitimoli
Copy link

mgtitimoli commented Aug 13, 2018

I believe this is related with the fact that setTimeout implementation in node is not the same as in web, that in the latter you will get a number back.

So for this kind of cases it will be nice to be able to specify the environment in flowconfig, so this way flow would be able to expose the correspondent libs that apply for it, that is what eslint does to resolve the same issue (see here).

In any case, and going back to this specific issue, we should be getting back a number, and this is not the case either, I had a look to the source and I found that's exposed as an opaque type (see here), can you confirm this is also happening for you @gajus?

@gajus
Copy link
Author

gajus commented Aug 13, 2018

I believe this is related with the fact that setTimeout implementation in node is not the same as in web, that in the latter you will get a number back.

That is correct. In DOM, you are going to get back a number. In Node.js, it is an object.

So for this kind of cases it will be nice to be able to specify the environment in flowconfig

This would make sense. Though, how would two projects with different environments interact with each other?

can you confirm this is also happening for you @gajus?

Referring to what?

@mgtitimoli
Copy link

mgtitimoli commented Aug 13, 2018

how would two projects with different environments interact with each other?

hm, I haven't taken into account this possibility, but I believe it could potentially be handled by adding support for nested configs.

Referring to what?

That TimeoutID should be a number and it's not, but after asking you this I realized the reason why we are not getting number is because it's declared as an opaque type.

@lll000111
Copy link
Contributor

lll000111 commented Aug 20, 2018

The opaque type makes sense because of the mentioned cross-platform compatibility problem, since it hides the implementation details.

I think if you have node.js only code and want to use the timeout object's functions you could (should, IMO) define your own TimeoutId type and use that instead of the one from the Flow lib. After all, your code is incompatible with other JS runtimes. Use the Flow lib (opaque) type when you don't use platform specific extensions but only "standard Javascript" timer functions.

@goodmind goodmind added the Library definitions Issues or pull requests about core library definitions label May 17, 2019
@gajus
Copy link
Author

gajus commented Jul 22, 2019

I think it would be nice if Flow exported a stdlib for different environments. Could be a community project too, e.g.

import {clearTimeout, setTimeout} from 'flow-stdlib/node';

Only for cases where Node.js/ browser have non-standard features, such as this.

@goodmind
Copy link
Contributor

@gajus This just seems strange, ideally node.js would be shipped in flow-typed, not in Flow core, so you would have option to uninstall it

@goodmind
Copy link
Contributor

Or just this PR #7732

@gajus
Copy link
Author

gajus commented Jul 22, 2019

The benefit of this approach is that it would make it a clear distinction which dependencies are designed to work in Node.js and which are designed to work in browser.

I dig the simplicity of your PR too, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Library definitions Issues or pull requests about core library definitions
Projects
None yet
Development

No branches or pull requests

4 participants