Add developer tools extension. by teymour-aldridge · Pull Request #1094 · yewstack/yew

teymour-aldridge · 2020-04-20T13:04:28Z

This (draft) pull request implements feature #1090. It opens a WebSocket connection using a thread_local! and sends information about components to a (not yet developed) browser extension to make it easier to debug Yew applications.

jstarry · 2020-04-20T14:26:05Z

@teymour-aldridge could you please rebase off latest master. CI times have been reduced and repo structure has been updated

jstarry

@teymour-aldridge could you please rebase your changes off of master? There are a lot of changes in this PR that are unrelated to dev tools (I think from a bad merge?)

teymour-aldridge · 2020-05-03T07:41:32Z

Yes. My git skills are not the best. I’ll try to rebase again.

…

On 3 May 2020, at 05:44, Justin Starry ***@***.***> wrote: @jstarry commented on this pull request. @teymour-aldridge <https://github.com/teymour-aldridge> could you please rebase your changes off of master? There are a lot of changes in this PR that are unrelated to dev tools (I think from a bad merge?) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1094 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKFSTPI7UTUXRTIELMR5KDTRPTZEPANCNFSM4MMNSGXQ>.

jstarry · 2020-05-03T09:09:12Z

@teymour-aldridge good luck! If you can't get it working lmk and I can fix it for you

teymour-aldridge · 2020-05-09T15:03:48Z

@jstarry I think I've now done this successfully. If I haven't, would you mind letting me know?

jstarry · 2020-05-10T02:48:06Z

+pub mod dev;
+
+#[cfg(feature = "dev")]
+thread_local! {


nit: can you please move this inside the dev module? I don't think it needs to be here

I agree, I'll move it into the dev module.

jstarry · 2020-05-10T02:49:44Z

    Chunk(Option<FileChunk>),
    Files(Vec<File>, Chunks),
    ToggleByChunks,
+    InvalidString,


The changes in this file can be removed. In a separate change, I changed the type of Chunk to use Option<FileChunk>

jstarry · 2020-05-10T02:50:00Z

  "examples/two_apps",
  "examples/webgl",
-]
+]


nit: please add a newline to remove this change

jstarry · 2020-05-10T02:50:25Z

 serde = "1"
 serde_derive = "1"
-yew = { path = "../../yew" }
+yew = { path = "../../yew", features = ["std_web"] }


nit: revert this change

jstarry · 2020-05-10T02:50:35Z

    </head>
    <body>
-        <script src="/todomvc.js"></script>
+        <script src="/todomvc_std_web.js"></script>


nit: revert

jstarry · 2020-05-10T03:46:47Z

    pub fn mount(self, element: Element) -> ComponentLink<COMP> {
+        #[cfg(feature = "dev")]
+        let _ = wasm_bindgen_futures::spawn_local(async move {
+            let new_debugger = crate::dev::DebuggerConnection::new().await;


I don't think this is necessary if you have a message queue. The debugger connection message queue should be immediately available

No – it's here from before I added the message queue.

jstarry · 2020-05-10T03:47:39Z

+
+/// Describes the state of a WebSocket connection.
+#[derive(Debug)]
+pub enum ConnectionState {


Is this used?

No (and the future isn't being used either). It's some code I had here when I was trying to work out how to handle the WebSocket connection.
I'm now using an event listener to send messages when the connection opens (I'll also add one to raise an error if it can't connect).

jstarry · 2020-05-10T03:48:47Z

+    /// The event which has happened
+    event: ComponentEvent,
+    /// Optional additional data about the event.
+    data: Option<DebugComponent>,


Why would this ever be None?

jstarry · 2020-05-10T03:51:26Z

+/// A message sent to describe a change in a component's state.
+#[derive(Serialize, Debug)]
+pub struct ComponentMessage {
+    /// Time in seconds since the page was loaded.


Should be milliseconds, right?

jstarry · 2020-05-10T03:51:59Z

+#[derive(Serialize, Debug)]
+pub struct ComponentMessage {
+    /// Time in seconds since the page was loaded.
+    time: f64,


What is the purpose of sending this?

I thought it could be used to build a timeline of events on the page.

The extension will receive messages in order so I think it can handle timestamping on its end

That makes sense, I'll remove the field.

jstarry · 2020-05-10T03:53:15Z

This is a good first step, let me know if you need any clarification on my comments!

totorigolo · 2020-05-12T19:26:36Z

I love this PR. Did you start working on the browser extension?

teymour-aldridge · 2020-05-12T19:29:31Z

This PR (I love it too!) is a mess right now. I'm trying to fix it, but once that's done I intend to start working on the browser extension.

My thoughts on the browser extension are that hopefully I'll be able to get the extension to open a WebSocket server, but I suspect that a server running locally coordinating the extension and the code may be necessary which would in turn require a command line script.

I'm not sure when this will be releasable.

This function generates strings which can be used to identify DOM nodes.

They are now two different traits.

Co-authored-by: Justin Starry <justin.m.starry@gmail.com>

teymour-aldridge force-pushed the devtools branch from eae9256 to 536707c Compare April 20, 2020 15:02

teymour-aldridge marked this pull request as ready for review May 1, 2020 07:46

jstarry reviewed May 3, 2020

View reviewed changes

jstarry reviewed May 10, 2020

View reviewed changes

teymour-aldridge marked this pull request as draft May 11, 2020 07:43

teymour-aldridge added 18 commits May 13, 2020 11:57

Added missing Component::change impl.

1b5d691

Fixed a type mismatch.

8eb0b71

Rebase branch from master.

949c39e

Added more send_message calls.

a90066f

Rebased from master.

e375e75

Added selector function to generate selectors.

48e598f

This function generates strings which can be used to identify DOM nodes.

Remove 'dev' feature from todomvc.

b56ceb9

Added tests for 'dev' feature to CI.

201a7d4

Add selectors for DOM elements.

990cd4c

Added missing Component::change impl.

cfa99f6

Fixed a type mismatch.

86f8ef2

Added WebSocket interface to developer tools.

27132af

Added more send_message calls.

803b474

Added stdweb support.

fedf9e6

Added selector function to generate selectors.

39b7070

This function generates strings which can be used to identify DOM nodes.

Remove 'dev' feature from todomvc.

d52a86c

Add selectors for DOM elements.

b3dac7e

Fix some problems from rebase.

56523e4

teymour-aldridge force-pushed the devtools branch from 2f22368 to a78032d Compare May 13, 2020 11:04

teymour-aldridge and others added 17 commits May 13, 2020 12:04

Added integration tests.

4c08d88

Added await for WebSocket connection.

6759674

Switched DebuggerConnection to use a queue.

899192a

Separated debugger sending and queuing.

44f9f05

They are now two different traits.

Updated test to include message sending.

6c2a297

Added future for DebuggerConnection

9faf35e

Added an additional test for DebuggerConnection.

6d9542b

Included tests for dom selector generator.

d1e18b3

Made integration tests async.

97cdc7c

Added missing assertion in the integration test.

29d601c

Added event listener for WebSocket opening.

203be22

Use unwrap_or instead of match

22e11cd

Co-authored-by: Justin Starry <justin.m.starry@gmail.com>

Added function send_message.

0287ba0

Updated queue_message calls to call send_message.

2fbff54

Move some error handling into event listener.

176772d

Remove unnecessary future.

2896c20

Tidy up a test.

a78032d

teymour-aldridge closed this May 13, 2020

Uh oh!

Conversation

teymour-aldridge commented Apr 20, 2020

Uh oh!

jstarry commented Apr 20, 2020

Uh oh!

jstarry left a comment

Choose a reason for hiding this comment

Uh oh!

teymour-aldridge commented May 3, 2020 via email

Uh oh!

jstarry commented May 3, 2020

Uh oh!

teymour-aldridge commented May 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jstarry commented May 10, 2020

Uh oh!

totorigolo commented May 12, 2020

Uh oh!

teymour-aldridge commented May 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

teymour-aldridge commented May 12, 2020 •

edited

Loading