The last two days I've spent quite some time on a React+Shoelace based app.
To be honest: That was exactly 0% fun ๐
It's currrently way too much work to prepare your Shoelace React components to work properly.
I had to deal with subtle bugs in three different projects just for that little app.
We really really really really need an official shoelace package that provides working React components for all Shoelace custom elements out of the box.
Requirements:
displayName, support for refs etc.)I think it would make sense to auto-generate the wrapper components by a script.
See:
node_modules/@shoelace-style/shoelace/dist/metadata.jsonOf course this could be done as an inofficial third-party project ... but frankly, who likes to write professional apps using inofficial UI bindings?!? ... it really should be an official Shoelace sister or sub project.
[Edit] Please find here a related issue that I've opened today.
I agree that there are pain points with React and we could be doing something to make Shoelace work better with it. I have opinions about React and its reluctance to support web components despite how ubiquitous they've become, but I'll set that aside for now. Maybe we can set an example for how web component libraries _can_ work smoothly with React, even if it requires a shim.
That said, this should probably be done with @lit-labs/react. I believe it can be automated since we have all the necessary info in metadata.json. The Shoelace React wrapper can probably be archived once that happens.
This will probably start as a separate, experimental package because I don't think framework-specific things should be part of the main library. I can see it being called @shoelace-style/react.
@claviska
@shoelace-style/react would be great ... "experimental" would also be okay in the beginning ... but as soon as it turns out to be working, an "officially supported" state would be great. Like written above, you normally cannot (or should not) write apps for customers based on "inofficial" or even "experimental" libraries that may or may not be maintained in future.
I really think an official React support would be a great selling point for Shoelace.
And "official" does not necessarily mean that YOU have to implement or maintain it always personally. Maybe two members of the Shoelace community could take care (not me of course, somebody else :wink:).
I hoped once that "Shoelace react component" project is implemented everything can work most of the time automatically by just calling something like:
> ncu -u
> yarn install
> yarn build
Okay, I gave it a try in the "theme designer" project (which is react based).
At the moment the auto-generated react component file looks like follows, just to catch a brief glimpse (be aware that this is currently just a not-necessarily-quick-but-indeed-dirty proof of concept):
https://github.com/mcjazzyfunky/shoelace-theme-designer/blob/master/src/main/components/shoelace/generated-react-components.ts
The metadata.json file is a great help - the react component generator script is basically "reading metadata from file -> converting the metadata to a string of generated source code -> saving the generated source code to the output file".
I've not done the part with the events, yet, but it's really a good thing that currently in Shoelace events are defined as for example here, means a public property for the event emitter and a strict naming convention for the event name and the corresponding property name (@claviska: would be great if this convention will also be used in future). This allows to retrieve the event's detail type.
Sounds promising! I plan on following a similar approach, but generating a separate file for each component to avoid tree-shaking issues.
I've not done the part with the events, yet, but it's really a good thing that currently in Shoelace events are defined as for example here, means a public property for the event emitter and a strict naming convention for the event name and the corresponding property name (@claviska: would be great if this convention will also be used in future). This allows to retrieve the event's detail type.
I plan on following that convention, but these are also available in metadata.json along with a type for event.detail.
these are also available in metadata.json along with a type for event.detail
Ah yeah, stupid me ... this is much better ... although event.detail should not use any only locally available type information, but that's currently not the case. I'll go that way, when I'll continue with that project. Thanks for the hint.
If there's anything I can do to help enable this, let me know. I'd be happy to provide support to get this up-and-running, maintaining, or documenting
I threw a beta up here: https://www.npmjs.com/package/@shoelace-style/react
Full instructions are in the readme. Please try it out and let me know how it works or if you run into any problems. ๐
If everything looks good, I'll update the official docs to point to this package and deprecate @shoelace-style/react-wrapper, as this uses @lit-labs/react which is better than my makeshift wrapper.
Super super minor problem with the docs on that page:
import from '@shoelace-style/react/dist/button';
import '@shoelace-style/react/dist/spinner';
import from ' is not valid syntax, we'd want to drop the from
Thanks. Just pushed a fix and published beta.2 to update it.
Note: I've accidentally opened this issue as the wrong user (using the "wrong" browser). That's annoying, I normally do not use this @js-works user for communication, so I switch to @mcjazzyfunky from now on - hoping not to confuse anybody.
Thanks @claviska for the implementation.
I've tested it with the theme designer. Was working fine :+1:
But nevertheless there are some open issues:
onmouseover vs. onMouseOver difference.Auto-loading/-registering of the depending shoelace custom elements should also be considered a must-have, IMHO (if anybody is using the sl-button without sl-spinner then the resulting package is a bit larger then absolutely necessary ... so what?! ... everything else would be premature optimization at cost of DX, IMHO). Something like the following could do the job, I guess:
import * as React from 'react';
import { createComponent } from '@lit-labs/react';
export function createReactComponent(tagName, elementClass, events, dependencies) {
// argument `dependencies` (an array) will be ignored, it's only purpose
// is to prevent tree-shaking issues
return createComponent(React, tagName, elementClass, events);
}
I had some issues on DEV system with lots of errors similar to Can't resolve '@lit-labs/react' in 'C:\...\shoelace-theme-designer\node_modules\@shoelace-style\react\dist\tab'. When I've reconfigure my webpack dev config from "mode": "development" to "mode": "production" then everything worked fine. I guess, the problem is on my side, but maybe, Cory, you could also check whether the dependencies config in that @shoelace-style/react project is exactly as you want it to be ... just to make sure .... thanks.
We really need the proper TS types, this is without any doubts a must-have. Maybe including this onmouseover vs. onMouseOver difference.
I agree. I'm not sure how to achieve that with the wrapper if it doesn't come out of the box. I'm not really willing to manually type every prop for React. I hope @justinfagnani doesn't mind me pinging him for some guidance here. ๐ฌ
Auto-loading/-registering of the depending shoelace custom elements should also be considered a must-have, IMHO
While it would be nice, I don't think it should come at the wrapper level. This behavior, although somewhat inconvenient, is the same whether you're cherry picking modules, loading via webpack, or using the React wrapper.
I would consider it out of scope for _this issue_, but not for the library in general. I'm just not sure how it can be achieved at the moment so it will have to come later.
I had some issues on DEV system with lots of errors similar to Can't resolve '@lit-labs/react' in 'C:...shoelace-theme-designernode_modules@shoelace-stylereactdisttab'.
It's a standard dependency, but I wonder if the * is causing that.
"dependencies": {
"@lit-labs/react": "*"
}
The package is experimental, which is why I consider this package to be experimental as well. Once it reaches a stable version number, I'll pin it to major.
Regarding types, I'm pointing the React wrapper at the dist folder which contains the esbuild output along with the corresponding .d.ts files for each component. I figured TypeScript would find them automatically, but maybe something needs to be done to point the wrapped component to the appropriate type file.
I agree. I'm not sure how to achieve that with the wrapper if it doesn't come out of the box. I'm not really willing to manually type every prop for React.
@claviska
I don't think that manual typing will be necessary. My written-far-too-late-at-night proof-of-concept surely has a lot of flaws, but the typing itself was not completly bad, i guess (prop and method types are derived from the corresponding Shoelace custom element class itself):
Types have been added and will be available in the next release of @shoelace-style/react. It's currently waiting on a new release of @lit-labs/react, but I'm told it will be available soon.
Details: https://github.com/shoelace-style/react/issues/1
I'm going to start tracking improvements to the React package on that repo, so closing this since we have a working implementation.
Going back to the initial objectives for clarification:
You import the required Shoelace react component and it will work out-of-the-box.
We've met this objective, but Shoelace and React are peer dependencies. This seems to be the best approach, since we don't want to lock users down to specific Shoelace + React versions nor distribute those packages through the wrapper.
The Shoelace react components shall be properly typed (including event handling)
Finally nailed this down, but still waiting on a dependency update before pushing it live. Track it here.
All depending Shoelace elements must be registered automatically when using a Shoelace react component.
This isn't a React dist problem, but a general dependency problem that needs to be solved at a higher level. It's out of scope for this feature, but it does make sense for a more general discussion.
Components (and depending elements) shall be tree-shakeable
Each component is wrapped and released in its own file (i.e. it's not bundled), which then includes directly from @shoelace-style/shoelace, which is tree-shakeable.
The Shoelace react component should behave as close as possible to a "normal" react component (regarding displayName, support for refs etc.)
Thankfully, this was handled by offloading it to @lit-labs/react, so additional alignment would be a feature request there. They have a large user base and I'm told the wrapper is used extensively in a handful of larger orgs, so all the stuff you'd should "just work" by now.
@claviska Many thanks for your work. Is it possible that https://github.com/shoelace-style/react is currently a private repository or whatever? At least it results in a 404...
For some reason it was. Must have ticked it by accident when I set it up. It was intended to be public from the start. ๐คฆ๐ปโโ๏ธ
Most helpful comment
I threw a beta up here: https://www.npmjs.com/package/@shoelace-style/react
Full instructions are in the readme. Please try it out and let me know how it works or if you run into any problems. ๐
If everything looks good, I'll update the official docs to point to this package and deprecate
@shoelace-style/react-wrapper, as this uses@lit-labs/reactwhich is better than my makeshift wrapper.