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

test: test ModuleRunnerTransport invoke API #18865

Merged

Conversation

dario-piotrowicz
Copy link
Contributor

This PR is adding a test for the invoke method of the ModuleRunnerTransport

Followup from: #18851

@dario-piotrowicz dario-piotrowicz force-pushed the dario/runner-transport-invoke-test branch from 8c6af6e to bf534db Compare December 3, 2024 00:45
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 🙏

dario-piotrowicz and others added 6 commits December 3, 2024 10:05
use different broadcast channel name

Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>
use birpc to avoid relying on internal structure of invoke
@sapphi-red
Copy link
Member

Thanks for the change 💚

While trying this PR locally, I found a bug in the previous PR and made a fix for it (#18876).
Alongside that, I pushed some commits to this PR.

72ee49b: set the return value of invoke on the main thread side as it will be called from the worker side, also made the successful case to use hot.handleInvoke
0a1db74: removed the transport, as in this case, transport is not used

The tests should pass after #18876 is merged.

@dario-piotrowicz
Copy link
Contributor Author

Thanks a lot for the changes @sapphi-red 🙏

@sapphi-red sapphi-red added test feat: environment API Vite Environment API labels Dec 6, 2024
@sapphi-red sapphi-red merged commit e5f5301 into vitejs:main Dec 6, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: environment API Vite Environment API test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants