Avoid circular dependencies between our craft/paint/render modules.
Review Request #13947 — Created June 5, 2024 and submitted — Latest diff uploaded
We had some pretty rough circular dependencies between
craft.ts
,
paint.ts
, anddom.ts
. The crafting code would need to paint, and the
paint could would sometimes need to craft. The paint could would also
need to callsetProps
fromdom.ts
, but that module also had
renderInto
, which needed to craft.We're taking the following approach to resolving this issue:
dom.ts
is now split into sub-modules for different purposes.
props.ts
handles property management, andrender.ts
handles
render logic.dom/index.ts
wraps both.The main, co-dependent craft/paint machinery now lives in a
_craftAndPaint.ts
module. Some of this is private, and some public.
craft.ts
andpaint.ts
re-export these along with their purely
public-facing methods.This fully avoids the nasty circular dependencies and makes areas of
responsibility a bit more clear. Mostly.There aren't any logic changes anywhere. This change is entirely code
motion.Rollup still warns us about other circular dependencies, but these
should be ignored. These are cases where we have anindex.ts
that
includes a module that eventually includes another module. The resolver
is going back throughindex.ts
as part of module resolution, and
that's tripping up Rollup. This seems to be documented in Rollup.js
ticket #2271 (https://github.com/rollup/rollup/issues/2271), and at the
moment there are no good options. Fortunately, it's not considered a
dangerous circular dependency.
Unit tests passed.
Builds passed.
Storybook ran as expected.