feature #76: build ui-vue as web component as well #106
Conversation
| @@ -26,9 +26,10 @@ | |||
| "yarn": "1.22.19" | |||
| }, | |||
| "scripts": { | |||
| "build": "npm-run-all -nl build:*", | |||
| "build": "npm-run-all -nls build:*", | |||
packages/ui-vue/src/index-wc.ts
Outdated
| interface WebComponent { | ||
| styles: string[]; | ||
| emits: string[]; | ||
| } | ||
|
|
||
| const OdkWebFormWC = OdkWebForm as unknown as WebComponent; |
There was a problem hiding this comment.
Help needed here, can't figure out the type of OdkWebForm
There was a problem hiding this comment.
Really briefly (reiterating here, since we also chatted a bit about this earlier when we talked about the last change in #114)... this is a great example of what I've been talking about when I say that the various tools (ESLint, Vue, TypeScript, typescript-eslint... and all of their respective parsers that are meant to work together) fail to agree on whether a .vue component module does or does not have types.
The feedback that I had written was along these lines:
This will hopefully become less of an issue, when some tooling improvements are fully available/when we're able to update the affected tooling.
Until that's resolved, I believe we should take a two pronged approach to the issue:
-
When we're merely importing a
.vuecomponent into a.tsmodule, and then passing it as a value to something else, I think we can generally accept the hit to type safety of a cast. This is what we've done so far. -
When we're actually interacting with the value of an imported
.vuecomponent (e.g. by accessing its properties as we are here, and passing them to APIs which expect them to be of a certain type)... I think we should take some more care to validate their types rather than casting them. Common approaches to this use type predicates and/or assertion functions.
I also took some time, before posting those thoughts, to prototype some ideas around how we could do (2) in a general/reusable way that shouldn't be too onerous. I had an approach that felt like it was going in the right direction (stashed now, we can revisit). So I applied the concept to an import of OdkWebForm.vue, performing a runtime validation that the component does actually have an emits property which is an array of strings, and a styles property which is an array of strings.
But... the component does not have a styles property at all. All of the tooling issues aside, this type cast is incorrect.
My next hunch was that maybe that property becomes populated if a component specifically includes a <style> block. It doesn't in this PR, but I thought maybe it came up with #113 which does add a style block there. Unfortunately, at least adding a quick <style> block locally, I still do not see styles: string[] on the imported component. I also do not see anything else on the component which might provide the same data.
I don't really know what to suggest here. I don't know what the type should actually be, or what styles is supposed to contain.
There was a problem hiding this comment.
I have pushed a new commit which simplifies this a lot by taking your suggestion from the slack message:
it looks like the body of the custom element definition's setup is analogous to a Vue component. i think...? would it maybe make sense if it were also in a .vue module
There was a problem hiding this comment.
Regarding missing styles property; it is the magic done by @vitejs/plugin-vue. When a .vue component is consider as custom element (by default when file extension is .ce.vue) then the plugin extracts the style text and adds it to the component. See this https://vuejs.org/guide/extras/web-components.html#sfc-as-custom-element
packages/ui-vue/vitest.config.ts
Outdated
| export default defineConfig((options) => { | ||
| return mergeConfig( | ||
| viteConfig(options), | ||
| defineConfig({ | ||
| test: { | ||
| environment: 'jsdom', | ||
| exclude: [...configDefaults.exclude, 'e2e/*'], | ||
| root: fileURLToPath(new URL('./', import.meta.url)), | ||
| // Suppress the console error log about parsing CSS stylesheet | ||
| // This is an open issue of jsdom | ||
| // see primefaces/primevue#4512 and jsdom/jsdom#2177 | ||
| onConsoleLog(log: string, type: 'stderr' | 'stdout'): false | void { | ||
| if (log.startsWith('Error: Could not parse CSS stylesheet') && type === 'stderr') { | ||
| return false; | ||
| } | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| ); | ||
| }) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Just wrapped in defineConfig because viteConfig is now defined with callback
sadiqkhoja
force-pushed
the
features/76-web-component
branch
from
75c9575 to
3b4b010
Compare
|
We talked about deferring this for now, I think last week. Should we close for now and revisit when it's a priority? |
|
Adding a note for future reference, avoiding the Shadow DOM is now possible as of Vue v3.5 (released 5 months ago): defineCustomElement(OdkWebFormCE, {
shadowRoot: false,
})
Either way, a Web Component wrapper may be viable now 😄 |
Closes #76
Changes:
index-wc.tsis the entry point for the Web Component. It does all the shenanigans to install plugin, copying styles to the document root, etc.Challenges faced:
The simplest solution for above two challenges is to not use shadow DOM at all, but currently Vue doesn't support it, there is an open PR for that: vuejs/core#4404
I have gone with the option of dynamically add the styles defined in the shadow DOM to the head of the host application. So on the runtime same set of styles are inside of the shadow DOM as well as at the root of the root. - not the cleanest thing :( but at least there is no additional load on the network.