Skip to content

[poc] add static stencil component#234

Merged
janniks merged 1 commit intomainfrom
poc/add-show-inline
Jun 15, 2022
Merged

[poc] add static stencil component#234
janniks merged 1 commit intomainfrom
poc/add-show-inline

Conversation

@janniks
Copy link
Copy Markdown
Contributor

@janniks janniks commented Jun 2, 2022

  • adds dist-custom-elements distribution of connect-ui, since rollup-based projects don't handle dynamic loaded stencil well
  • the new custom element is loaded (non-dynamically) in the new showConnectStatic method (just a copy of showConnect without dynamic loading)

using *Static would always bundle the connect-ui (ideally only shown when somebody has an incompatible browser, or is missing the wallet extension) into a project. however, developers could adjust their configs (e.g. rollup > manualChunks) to achieve similar dynamic loading/chunking, but handled by the last bundler in the build step (rather than stencil before distribution, which is optimized for webpack)

@janniks janniks force-pushed the poc/add-show-inline branch 2 times, most recently from 7dafc4e to 71fc095 Compare June 2, 2022 01:29
@janniks janniks force-pushed the poc/add-show-inline branch from 71fc095 to d22598b Compare June 2, 2022 17:49
@janniks janniks requested a review from kyranjamie June 2, 2022 17:57
@janniks
Copy link
Copy Markdown
Contributor Author

janniks commented Jun 2, 2022

cc @kyranjamie
does this make sense or are there other options to consider w/ stencil?

@kyranjamie
Copy link
Copy Markdown
Contributor

So the gist here is just creating an alternate method that synchronously loads the connect dialog?

Code looks good but, I'm not so familiar with stencil so have nothing specific to suggest.

@janniks
Copy link
Copy Markdown
Contributor Author

janniks commented Jun 3, 2022

So the gist here is just creating an alternate method that synchronously loads the connect dialog?

Yep, although upon re-checking the latest release literally just added a fix for this, days ago. It's marked as experimental, but I'll test in a new PR.

UPDATE: the other fix doesn't seem to work in the case. so i'm going to stick with the static option proposed here. if the issue in stencil is resolved we can retry moving to the experimental imports.

@janniks janniks marked this pull request as ready for review June 15, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants