Conversation
This reverts commit e901369.
|
@nikku Resurrecting this a little bit to make a point. Properties panel is a component library. And the way component libraries should expose preact or react is well agreed to be as a peer dependency, not a direct dependency. This is something that is quite well agreed in the common consciousness (stack overflow, AIs, blogs, other component libraries like carbon). Why we can do it: properties panel exports JSX directly, it doesn't have the responsibility of rendering. Since it's designed to be rendered as part of an upstream library, there will never be a scenario where a consumer doesn't itself have to provide preact. Why we should do it: peer dependency mismatches will error at package install time, regular dependency mismatches will cause package duplication -> runtime errors. I know de-duplication works well but this is about the scenarios where it doesn't. The way I see it, we should have:
I should note that as of now, form-js and properties-panel are not compatible from a preact semver point of view. We are doing some heavy trickery to bundle the properties panel inside of form-js and aliasing all of the preact deps inside of properties panel (which point to the bundled copy) back to form-js top level. Effectively overriding the copy entirely. It works for now but the moment the properties panel depends on a 10.16+ feature of preact we won't be able to upgrade anymore. (This is what initiated me to resurrect these talks). (PS the fact that form-js doesn't support preact 10.16.0 and above is also a real issue that needs tackling, but serves as an example of how having a component library opinionated about preact versions can cause upstream issues) |
|
@Skaiir Preact used in properties panel should be an implementation detail, to properties panel and everyone who extends it. Why do you want to use the same The issue with peer dependencies is that you can easily get into the situation where you have a version of a library (let's say preact@11) that properties panel is not compatible with - boom, you will not be able to use properties panel anymore - a nested dependency enforces what version of Does that make sense? Am I missing something?
How will the use of |
|
I'd love to unbundle preact dependency, but at the same time marking it as a peer dependency enforces the version on the library consumer. |
I need to clarify something for myself, is the current "right" approach to do this? : |
|
That's what we do in bpmn-js-properties-panel: https://github.com/bpmn-io/bpmn-js-properties-panel/blob/main/src/render/BpmnPropertiesPanelRenderer.js#L174 |
This ensures
preactis not inlined anymore.Child of https://github.com/bpmn-io/internal-docs/issues/819