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

FinishedPromise.install() replaces native Promise #1

Merged
merged 4 commits into from
May 19, 2017

Conversation

aslakhellesoy
Copy link
Contributor

This is an attempt to provide a simple mechanism to replace the native Promise with FinishedPromise.

The rationale behind this is to be able to force 3rd party libraries using Promise to use FinishedPromise in tests.

I haven't tested this with any 3rd party libraries, so this PR is just to start the discussion. Do you think this would work?

@aslakhellesoy
Copy link
Contributor Author

I just remembered now what you told me earlier @joshski - the problem is with async await code - it's harder to make that use FinishedPromise ?

@joshski
Copy link
Member

joshski commented Apr 28, 2017

Yeah async/await is a problem that I'm not sure how to work around yet 😤

Also I wondered whether this was worth doing or not bearing in mind load order: a library may have already made a reference to Promise before FinishedPromise.install is called...

@aslakhellesoy
Copy link
Contributor Author

Relevant: nodejs/CTC#7

@aslakhellesoy
Copy link
Contributor Author

a library may have already made a reference to Promise before FinishedPromise.install is called

You mean stdlib library or 3rd party npm module?
I think it's possible to have FinishedPromise.install() before any other 3rd party npm modules are loaded - that could be done in the test runner's entry point coudn't it?

@joshski
Copy link
Member

joshski commented Apr 29, 2017

that could be done in the test runner's entry point coudn't it?

Sure, in theory. When I tried that in some early tests with mocha, I didn't have much luck. I forget exactly why, maybe it had something to do with the way mocha loaded before tests, or maybe it was some third-party library.

screen shot 2017-04-29 at 09 52 10

In the end, inconvenient as it may be, I think we may need to remember how to pass stuff in, if we are serious about separating the sync from the async :)

@aslakhellesoy
Copy link
Contributor Author

I think I've come up with an acceptable workaround for async - await. WDYT @joshski ?

@joshski
Copy link
Member

joshski commented May 2, 2017

Wow, great idea!

Looks promising 😆 -- will have a play with this as soon as I can

@joshski joshski merged commit 7571c1a into featurist:master May 19, 2017
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 this pull request may close these issues.

2 participants