[expo] introduce api for devtools plugin #24667
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 29, 2023
tsapeta
approved these changes
Oct 4, 2023
Kudo
force-pushed
the
@kudo/devtools-plugin-3
branch
from
c1d5448
to
7de36b2
Compare
Kudo
force-pushed
the
@kudo/devtools-plugin-3
branch
from
7de36b2
to
bf66a79
Compare
Kudo
changed the title
ENG-10329 Handle multiple devices workflow
Currently the devtool plugin clients broadcast messages on the |
brentvatne
approved these changes
Oct 18, 2023
There was a problem hiding this comment.
introduce useDevToolsPluginClient(pluginName) hook from the expo package
nice :) that simplifies the api.
as discussed last week, let's follow up at some point with some concept of a device id that is built into the client, so there is a first class concept built into this api for plugins to be able to handle multiple devices
tsapeta
reviewed
Oct 19, 2023
| */ | ||
| public abstract isConnected(): boolean; | ||
|
|
||
| protected handleMessage = (event: WebSocketMessageEvent): void => { |
There was a problem hiding this comment.
One nitpick: prefer using functions over arrow functions when the arrow function is not necessary 😉
There was a problem hiding this comment.
this is required because we want to bind "this" to the DevToolsPluginClient class. since the function would be called inside an event callback.
Kudo
added a commit
that referenced
this pull request
Oct 21, 2023
# Why support devtools plugins for autolinking close ENG-9981 having series prs to add devtools plugins support: - #24649 - #24650 - #24667 # How - add `devtools` as a platform - passing `projectRoot` rather than default implicit `process.cwd()` - exposing autolinking interfaces for cli integration - introduce `onlyProjectDeps` for not filtering out project dependencies from monorepo # Test Plan - ci passed - manual test from devtools plugins --------- Co-authored-by: Tomasz Sapeta <tomasz.sapeta@swmansion.com>
Kudo
added a commit
that referenced
this pull request
Oct 21, 2023
# Why add devtools plugins support to cli close ENG-9981 having series prs to add devtools plugins support: - #24649 - #24650 - #24667 # How - [prebuild-config] update expo-modules-autolinking integrate because now it exports public apis from #24649 - [cli] add `DevToolsPluginManager` and `DevToolsPluginMiddleware` for devtools plugins support # Test Plan add unit tests and ci passed
will follow up this in a separate pr and going to merge this. |
marklawlor
pushed a commit
that referenced
this pull request
Oct 30, 2023
# Why support devtools plugins for autolinking close ENG-9981 having series prs to add devtools plugins support: - #24649 - #24650 - #24667 # How - add `devtools` as a platform - passing `projectRoot` rather than default implicit `process.cwd()` - exposing autolinking interfaces for cli integration - introduce `onlyProjectDeps` for not filtering out project dependencies from monorepo # Test Plan - ci passed - manual test from devtools plugins --------- Co-authored-by: Tomasz Sapeta <tomasz.sapeta@swmansion.com>
marklawlor
pushed a commit
that referenced
this pull request
Oct 30, 2023
# Why add devtools plugins support to cli close ENG-9981 having series prs to add devtools plugins support: - #24649 - #24650 - #24667 # How - [prebuild-config] update expo-modules-autolinking integrate because now it exports public apis from #24649 - [cli] add `DevToolsPluginManager` and `DevToolsPluginMiddleware` for devtools plugins support # Test Plan add unit tests and ci passed
marklawlor
pushed a commit
that referenced
this pull request
Oct 30, 2023
# Why for devtools plugins integration close ENG-9981 close ENG-10329 having series prs to add devtools plugins support: - #24649 - #24650 - #24667 # How introduce `useDevToolsPluginClient(pluginName)` hook from the expo package. this api has some designs in mind: - pluginName as message namespace. so that method `hello` from different plugins will not conflict. - the websocket client in the app supports multiplexing. multiple plugins would have just one websocket client and will terminate the connection once reference count reaches zero. - if multiple webpages with the same pluginName connect to the app, we will terminate previous connection and let the latest plugin webpage win. # Test Plan add some unit test and ci passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
for devtools plugins integration
close ENG-9981
close ENG-10329
having series prs to add devtools plugins support:
How
introduce
useDevToolsPluginClient(pluginName)hook from the expo package. this api has some designs in mind:hellofrom different plugins will not conflict.Test Plan
add some unit test and ci passed
Checklist
npx expo prebuild& EAS Build (eg: updated a module plugin).