NEARBuilders / near-bos-webcomponent

Embed NEAR BOS content as a web component
The Unlicense
13 stars 6 forks source link

Upgrades to react-router 6.4 #6

Closed elliotBraem closed 8 months ago

elliotBraem commented 8 months ago

This upgrades react-router-dom to 6.4 and migrates usage. Looks like this now:

  const router = createBrowserRouter([
    {
      path: "/",
      element: (
        <Viewer
          widgetSrc={props.src}
          code={props.code}
          initialProps={props.initialProps}
        />
      ),
    },
    { path: "/*", element: <Viewer /> },
  ]);

  return <RouterProvider router={router} />;

Is also solves an issue I realized from previous PR that provided paths were not being supported outside default widget. Now you can do something like this again: http://127.0.0.1:3000/efiz.near/widget/Tree; previously it would still only show default widget.

petersalomonsen commented 8 months ago

How can we test this @elliotBraem ?

Maybe the time has come to introduce Playwright, so that we also can generate videos for PR reviews? Would be great to easily show a video demo of the enhancement here, in addition to that everything works as before with the upgrade of react router.

elliotBraem commented 8 months ago

I think that's a great idea -- but what do you mean by generating videos? Does playwright have this feature?

I'm actually getting testing started for potlock rn, just opened this PR: https://github.com/PotLock/bos-app/pull/488

I'm glad you brought this up, because it highlights a few things I've been trying to understand.

Also, I've noticed that bos-workspace can replace the scripts/dev-gateway from devhub repo, we can also specify dist to use with -g flag.

petersalomonsen commented 8 months ago

I think that's a great idea -- but what do you mean by generating videos? Does playwright have this feature?

Yes it is a simple boolean flag in the playwright config. Just see the videos in here: https://github.com/NEAR-DevHub/neardevhub-bos/pull/691#discussion_r1534219427

I'm actually getting testing started for potlock rn, just opened this PR: PotLock/bos-app#488

That is awesome! Happy to see more testing for projects in the ecosystem. I think this makes it so much easier to demonstrate and document pull requests, and also reduce the fear of breaking things.

I'm glad you brought this up, because it highlights a few things I've been trying to understand.

Also, I've noticed that bos-workspace can replace the scripts/dev-gateway from devhub repo, we can also specify dist to use with -g flag.

That's nice ! Can you develop with this without having to use localstorage flags for the bos-loader? you maybe don't need the bos-loader at all when using bos-workspace?

elliotBraem commented 8 months ago

Added tests @petersalomonsen

Default w/ intitialProps: https://github.com/NEARBuilders/near-bos-webcomponent/assets/16282460/1c8e09f1-efd0-4ff1-b62c-eb52d4d0cba7

Other paths w/ params: https://github.com/NEARBuilders/near-bos-webcomponent/assets/16282460/a6385611-1344-488e-8c3c-7a73eb7f5f57