Add hot reload #26

bb-face · 2024-06-20T21:51:10Z

PR for the issue: #20

Videos of the test:

redirectmap-hot-reload-If-enablehotreload-is-true-an-api-request-to-websocket-io-will-be-triggered-chromium.mp4

redirectmap-session-storage-Should-be-possible-to-provide-a-redirectmap-through-session-storage-key-chromium.mp4

redirectmap-hot-reload-If-enableHotReload-is-false-there-will-be-no-api-request-to-socket-io--chromium.mp4

redirectmap-bos-loader-url-Should-be-possible-to-provide-a-redirectmap-through-flags-in-localStorage-chromium.mp4

elliotBraem · 2024-06-21T16:37:21Z

src/App.js


-const SESSION_STORAGE_REDIRECT_MAP_KEY = 'nearSocialVMredirectMap';
+export const SESSION_STORAGE_REDIRECT_MAP_KEY = "nearSocialVMredirectMap";


Let's move this to useRedirectMap rather than exporting

elliotBraem · 2024-06-21T16:57:04Z

src/contexts/hotReloadContext.js

@@ -0,0 +1,3 @@
+import { createContext } from "react";
+
+export const HotReloadContext = createContext(false);


Since this is all this is, what if we cleaned it up some -- we could combine hooks/useRedirectMap and contexts/hotReloadContext

maybe into utils/RedirectMap? or dev-tools/RedirectMap?

This could export the useRedirectMap hook, and export a RedirectMapProvider (instead of doing </HotReloadContext.Provider> in App.js) -- and have it take "enableHotReload" as a prop instead of "value".

yes, shouldn't hot reload be configurable in some other way than directly in the source here? With this approach, how can hot-reload be configured when using the near-bos-webcomponents npm package?

@elliotBraem I tackle your comment in the last commit: 4eff901

petersalomonsen

Quite a bit of changed formatting of the code, without any real changes. And also added lines with comments. A bit confusing when reviewing and trying to find the actual change.

Would it be possible to make the diff a bit clearer on what has actually changed?

petersalomonsen · 2024-06-22T02:47:40Z

Also see my comment here about the redirect map

#20 (comment)

petersalomonsen

Would it be possible to add a test for triggering the hot reload?

petersalomonsen · 2024-06-22T13:14:24Z

src/utils/redirectMap.js

+  useEffect(() => {
+    (async () => {
+      if (hotReload) {
+        const socket = io(process.env.BOS_LOADER_WS || "ws://127.0.0.1:8080", {


Is process.env available here in the browser context?

Also, must it be only 127.0.0.1? If developing in github codespaces, you get dedicated host names with https.

Fixed in this commit: e1af6c4

test structure WIP: websocket test WIP: hot realod websocket test Remove test

elliotBraem · 2024-06-28T16:16:09Z

bos-workspace is ready to support this:

NEARBuilders/bos-workspace#118

petersalomonsen

We also need the test for hot reload to ensure that what is implemented here will not be broken by someone else later. Also we need the test as a demonstration for how the hot reload works, and as a guide on how to use it.

src/utils/redirectMap.js

elliotBraem · 2024-07-03T23:44:27Z

@petersalomonsen yeah, good point on a test for validating hot reload.

Since the socket server for hot reload needs to be on the same port as the app, it's been difficult to write a test for this -- we've tried mocking web socket or proxying requests, but it was challenging -- I'm not sure, @bb-face may have gotten closer to figuring it out?

Regardless, I'm wondering if this test belongs in bos-workspace repository instead? Or maybe we should update this playwright webServer command to use bos-workspace dev -g ./dist instead of yarn start

petersalomonsen · 2024-07-04T09:02:56Z

@petersalomonsen yeah, good point on a test for validating hot reload.

Since the socket server for hot reload needs to be on the same port as the app, it's been difficult to write a test for this -- we've tried mocking web socket or proxying requests, but it was challenging -- I'm not sure, @bb-face may have gotten closer to figuring it out?

Regardless, I'm wondering if this test belongs in bos-workspace repository instead? Or maybe we should update this playwright webServer command to use bos-workspace dev -g ./dist instead of yarn start

yeah, this project does not have a dev-server ( and it was never the intention either), but bos-workspace does have that Is we want to build a development environment like bos-workspace in near-bos-webcomponent, then we should also introduce a server for serving the components and a websocket for hot reload.

The main purpose of this project is to be able to serve BOS content inside a custom element in any web page ( not just React apps ).

It's hard to introduce hot-reload without actually also taking over the role that bos-workspace has as a dev server. So not sure what was the intention behind introducing hot-reload here?

petersalomonsen · 2024-07-06T08:59:41Z

@bb-face @elliotBraem I read this again, and maybe I missed your point. Replacing the http-server with bos-workspace dev -g ./dist will probably give you the websocket served from the same port as you suggest. But it would still be good to have the test in this repo, since it feels a bit wrong to have functionality in here that we don't know how to test automatically.

bb-face · 2024-07-07T16:18:09Z

Hey @petersalomonsen! Thanks for the feedback, I just pushed a working test that in my opinion makes sure that hot-reload is works properly, let me know what do you think! The only problem left to fix is the port, unfortuntaely I couldn't find a way to leave the socket server run (during the test) on the same port as the web application but it's a requirement...if you have any suggestions they are more than welcomed!

elliotBraem · 2024-07-08T03:37:41Z

@bb-face, let's move this "enablehotreload" feature into a more generic attribute named "config". This generic attribute can support hot reload as well as other features we may want to configure in the futre, such as "enableComponentSrcDataKey", "defaultFinality", "commitModalBypass", "bypassTransactionConfirmation" which are all used by the VM and we may want to support on this roadmap. For example:

 {
    {
      dev: {
        hotreload: {
          enabled: true,
          wss: "wss://localhost:3001",
        },
      },
      features: {
        enableComponentSrcDataKey: true,
        commitModalBypass: {
          authorIds: ["mob.near", "root.near"],
          sources: [
            "cool.near/widget/CoolComponent",
            "awesome.near/widget/AwesomeComponent",
          ],
        },
        bypassTransactionConfirmation: true
        enableWidgetSrcWithCodeOverride: true
      },
      defaultFinality: undefined
    }
  }

I am in no way attached to this structure, but notice how hot reload has a customizable source, which we can use with the existing test -- otherwise it defaults to the same port. Don't implement the other features, only hot reload support.

…mponent into add-hot-reload

elliotBraem · 2024-07-09T18:48:54Z

This has been updated to use the config object with a test that confirms a working hot reload via socket server

…mponent into add-hot-reload

fixed now

bb-face added 4 commits June 20, 2024 10:49

Add hot reload

2a91549

Fix enableHotReload test

b76650b

Fix tests

30c9c1b

Fix tests

63cda36

bb-face assigned elliotBraem Jun 20, 2024

bb-face requested a review from petersalomonsen as a code owner June 20, 2024 21:51

elliotBraem reviewed Jun 21, 2024

View reviewed changes

elliotBraem removed their assignment Jun 21, 2024

petersalomonsen requested changes Jun 21, 2024

View reviewed changes

elliotBraem mentioned this pull request Jun 22, 2024

Support hot reload #20

Closed

bb-face added 2 commits June 22, 2024 14:13

Refactor

4eff901

Revert style changes

68b09a8

petersalomonsen requested changes Jun 22, 2024

View reviewed changes

petersalomonsen reviewed Jun 22, 2024

View reviewed changes

bb-face added 2 commits June 24, 2024 19:19

Fix socket address

d1a2839

Fix ws address

bcd6727

test structure WIP: websocket test WIP: hot realod websocket test Remove test

bb-face force-pushed the add-hot-reload branch from 12321ab to bcd6727 Compare June 26, 2024 21:43

elliotBraem added 3 commits June 27, 2024 12:51

update docs

8136a21

adds tests

f098c73

adds component

0f99176

elliotBraem requested a review from petersalomonsen June 28, 2024 16:04

Merge branch 'main' into add-hot-reload

007f6f5

elliotBraem mentioned this pull request Jun 28, 2024

Enable hot reload with near-bos-webcomponent NEARBuilders/bos-workspace#117

Closed

petersalomonsen approved these changes Jun 28, 2024

View reviewed changes

petersalomonsen previously requested changes Jun 29, 2024

View reviewed changes

elliotBraem reviewed Jul 2, 2024

View reviewed changes

src/utils/redirectMap.js Outdated Show resolved Hide resolved

elliotBraem mentioned this pull request Jul 2, 2024

Updates readme #31

Merged

4 tasks

WIP: working test on port 3001

f0371ee

Fix test, not time dependend now

d2ba5fa

bb-face and others added 7 commits July 9, 2024 15:02

Move hotReload to config

356578e

Single param for hotReload

81c8561

adds config object

caa55d0

Merge branch 'main' of https://github.com/nearbuilders/near-bos-webco…

e149502

…mponent into add-hot-reload

clean up

4eb2a27

fix readme

84a6ad1

formatting

425b02c

elliotBraem mentioned this pull request Jul 9, 2024

Introduce VM feature "enableComponentSrcDataKey" as a config option #37

Closed

3 tasks

elliotBraem added 2 commits July 9, 2024 18:12

Merge branch 'main' of https://github.com/nearbuilders/near-bos-webco…

1790cc0

…mponent into add-hot-reload

formatting

062c5d9

elliotBraem merged commit a3958f1 into main Jul 10, 2024
1 check passed

elliotBraem deleted the add-hot-reload branch July 10, 2024 23:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hot reload #26

Add hot reload #26

bb-face commented Jun 20, 2024

elliotBraem Jun 21, 2024

elliotBraem Jun 21, 2024

petersalomonsen Jun 22, 2024

bb-face Jun 22, 2024

petersalomonsen left a comment

petersalomonsen commented Jun 22, 2024

petersalomonsen left a comment

petersalomonsen Jun 22, 2024

bb-face Jun 24, 2024

elliotBraem commented Jun 28, 2024

petersalomonsen left a comment

elliotBraem commented Jul 3, 2024

petersalomonsen commented Jul 4, 2024

petersalomonsen commented Jul 6, 2024

bb-face commented Jul 7, 2024

elliotBraem commented Jul 8, 2024 •

edited

Loading

elliotBraem commented Jul 9, 2024


		const SESSION_STORAGE_REDIRECT_MAP_KEY = 'nearSocialVMredirectMap';
		export const SESSION_STORAGE_REDIRECT_MAP_KEY = "nearSocialVMredirectMap";

		@@ -0,0 +1,3 @@
		import { createContext } from "react";

		export const HotReloadContext = createContext(false);

Add hot reload #26

Add hot reload #26

Conversation

bb-face commented Jun 20, 2024

elliotBraem Jun 21, 2024

Choose a reason for hiding this comment

elliotBraem Jun 21, 2024

Choose a reason for hiding this comment

petersalomonsen Jun 22, 2024

Choose a reason for hiding this comment

bb-face Jun 22, 2024

Choose a reason for hiding this comment

petersalomonsen left a comment

Choose a reason for hiding this comment

petersalomonsen commented Jun 22, 2024

petersalomonsen left a comment

Choose a reason for hiding this comment

petersalomonsen Jun 22, 2024

Choose a reason for hiding this comment

bb-face Jun 24, 2024

Choose a reason for hiding this comment

elliotBraem commented Jun 28, 2024

petersalomonsen left a comment

Choose a reason for hiding this comment

elliotBraem commented Jul 3, 2024

petersalomonsen commented Jul 4, 2024

petersalomonsen commented Jul 6, 2024

bb-face commented Jul 7, 2024

elliotBraem commented Jul 8, 2024 • edited Loading

elliotBraem commented Jul 9, 2024

elliotBraem commented Jul 8, 2024 •

edited

Loading