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

feat(core): Add async mapping support with Promises #617

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

koenigstag
Copy link

@koenigstag koenigstag commented Apr 28, 2024

Description

As a developer, I want my async mappings to work, so I've added promise support.

Proposed Changes

Implement a new boolean parameter isAsync to indicate a request for async operations.

Add conditional return of Promise which does async mapping operations inside.

Add type safety for return value with typescript conditional types.

Checklist

  • Tests
  • Documentation

Copy link

sonarcloud bot commented May 4, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@koenigstag koenigstag changed the title feat(core): Add async mapping support feat(core): Add async mapping support with Promises May 4, 2024
@koenigstag
Copy link
Author

koenigstag commented May 4, 2024

Quoting the fake-async docs:

Real async support can be achieved by some Isomorphic Worker that would execute the map operations on the Worker thread

Can someone please explain - why do we need a Node.js Worker thread instead of Promise which are more common?

@vellotis
Copy link

Quoting the fake-async docs:

Real async support can be achieved by some Isomorphic Worker that would execute the map operations on the Worker thread

Can someone please explain - why do we need a Node.js Worker thread instead of Promise which are more common?

Hi @koenigstag. Great work for supporting the async/Promised approach!

My opinion is that the fake-async doesn't solve the problem. Long-living non-blocking IO operations may take a lot more time than the fake-async operation that is pushed to the (FIFO) event loop.

Your implementation is the appropriate solution, not the suggested fake-async one.

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