bpmn-io / form-js

View and visually edit JSON-based forms.
https://bpmn.io/toolkit/form-js/
Other
420 stars 109 forks source link

Draggle (dragula as well) doesn't play nice with preact v11.16.0 #760

Open Skaiir opened 1 year ago

Skaiir commented 1 year ago

Describe the Bug

We found a bug in the web modeler integration of form-js, where a forced update to preact 11.16 (via resolutions) made certain drag and drop operations lead to components disappearing from the editor. I was able to replicate this locally as well.

firefox_k3h2hnVXmR

While forced resolutions of dependencies is always something that can go wrong, this remains a problem for us to adopt newer versions of preact in the future.

The specific version of preact made some modifications to how children are diff'd. My guess is, that the draggle interaction with the DOM interferes with preact's new optimized diffing algo.

Steps to Reproduce

  1. put two components in a row next to eachother
  2. drag the right one to the left
  3. 🪄 field disappears

Expected Behavior

  1. field should be moved properly

Environment

Preact 11.16.0 only

pinussilvestrus commented 1 year ago

I created a draft PR + demo that makes this reproducible pretty easily: https://github.com/bpmn-io/form-js/pull/763

What is interesting is that also the type checks break.

CatalinaMoisuc commented 1 year ago

@marcosgvieira let's ensure we prioritize this for the 8.3 (October). release so we don't have to release a stable version of Web Modeler Self-Managed with an outdated version of preact. Thank you!

pinussilvestrus commented 1 year ago

My two cents on this one: even though we would find a fix to support newer preact versions with draggle, we should be careful when bumping to a newer version. We use the same version of preact in the properties panel, and I believe we should avoid having multiple versions around. As preact is a crucial part of our core libraries, migrating to a newer version should be a conscious decision across the stack. /cc @bpmn-io/modeling-dev

nikku commented 1 year ago

I believe preact@10 is still actively maintained, cf. latest version from August. So there is no immediate need to force upgrade in other applications (web modeler). If they do it, then they do it at their own risk, with certain consequences (cf. this issue).

In this sense I completely subscribe to https://github.com/bpmn-io/form-js/issues/760#issuecomment-1729185570.


What I propose is that web modeler uses the maintained (and known to be compatible) preact@10 version.

We'll at some point migrate our toolkits over to preact@11, in a controlled manner (https://github.com/bpmn-io/internal-docs/issues/822).

CC @CatalinaMoisuc