The Slot/Fill feature depends on a singleton registry in components package

[jsnajdr]

Steps to reproduce:

1. Create a plugin, e.g., wc-admin, which bundles the @wordpress/components package for better Core compat.
2. Export a Slot/Fill pair from that plugin.
3. Try to use it another plugin that doesn't bundle @wordpress/components

Actual result: The slots and fills will never materialize in the UI, because the wc-admin plugin has its own instance of a React context, different from the default one in wp-admin. Bundling @wordpress/components created a duplicate instance of the context, which effectively acts as a singleton registry for slots and fills.

More on the issue

The core issue seems to be that when @wordpress/components creates a React context by calling createContext and exports it, that context becomes a singleton that needs to be carefully managed. That singleton serves as a registry where all slot/fill pairs are registered by name. It becomes very similar to the @wordpress/data or @wordpress/hooks packages that also maintain a centralized registry of things. That obviously prevents them from being safely bundled.

Possible solution 1

The ultimate framework-level solution would be to remove that singleton, and make @wordpress/components export only the createSlotFill function, not an instance of the context. Each createSlotFill would create its own context (I think it would be even a performance improvement), managed by the specific package rather than by @wordpress/components library.

This, however, is a breaking API change, because we can no longer reference a slot by string name. A string name implies that it's a key into some registry. We can't avoid a singleton and have string names at the same time. A similar change was done by @gziolo recently in @wordpress/data: deprecating store names in favor of store objects.

The downside is also that createSlotFill would have to return a triplet: Slot, Fill and SlotFillContextProvider that connects them together. The app would have to render a potentially big number of context providers at the top, one for each slot/fill pair that it uses. That might prove untenable.

Or maybe it's possible to implement slot/fill without a context, making the slot and the fills communicate in some different, more direct way? Cc @diegohaz who is the architect of the latest "bubble-virtually" implementation. **Possible solution 2:**

Another solution would be to keep the singleton, but move the slot/fill feature to a separate package. Making it clear that it contains a singleton registry (like data or hooks), and therefore making components a bundle-able library again.

**Workaround:**

Reexporting the provider from the navigation package looks like a good workaround. Note that with the registry-less solution described above, we'd be forced to export the provider, too, because it's now an unique part of the API, together with the Slot and the Fill.

The issue was originally raised by @psealock in a private Woo-related discussion.

[psealock]

Another solution would be to keep the singleton, but move the slot/fill feature to a separate package

This appears to be a good solution. The reasons for WC Admin to bundle wp.components began as one of convenience but ultimately the decision is appearing to contribute to technical debt. We go to great lengths to offer Woo extensions entry points to extend WC Admin and offer up wp.components as a way to quickly create UIs. Should we ask extensions to follow our lead and also bundle the package? We already have commonly used extensions declaring wp-components as a dependency, so the entire package is loaded on the page twice (at least!).

The reason I'm saying this is because going through a major refactor for the purposes of letting consumers bundle wp.components seems like an action condoning less than optimal behaviour, Woo being guilty of this. The solutions proposed here should be considered on the merits of performance not ability to make the package bundle-able.

Having said that, moving SlotFill to its own package creates a nice temporary solution while we work to unbundle wp.components. Then again, the workaround of re-exporting a Fill, Slot, and Provider also works as a temporary solution. Are there other benefits to moving SlotFill to its own package?

[jsnajdr]

@psealock No matter whether WC Admin decides to unbundle @wordpress/components or not, I think moving SlotFill to a separate package is a sound architectural move, not just a temporary workaround for any urgent issue.

After separated, @wordpress/components and @wordpress/slot-fill will be two kinds of packages that are fundamentally different from each other. To illustrate, consider a plugin that uses two 3rd party packages in a code like this:

import { startsWith } from 'lodash';
import { addFilter } from '@wordpress/hooks';

addFilter( 'blocks.registerBlockType', 'enhance-core-blocks', ( settings, name ) => {
  if ( startsWith( name, 'core/' ) ) {
    settings.edit = enhancedEdit( settings.edit );
  }
  return settings;
} );

The plugin adds a filter that intercepts blocks registration and enhances the editor UI for core/ blocks somehow.

Here, the @wordpress/hooks package exposes some API of the underlying platform and lets the plugin influence something there. It's obvious that this package should never be bundled with the plugin. That just never makes sense. If it is bundled, the plugin simply stops working.

The lodash package is different. It's just a library of utilities. You don't need to bundle it, because lodash is a part of the WordPress platform and it's nice to share it with other plugins on the page. But if you decide you want to bundle it nevertheless, you can. Maybe you want to use a specific version with some new feature or bugfix. You will end up shipping more code, but the app still works.

Similarly, you might want to bundle @wordpress/components because you use features of a newer version, and don't want to break compatibility with older versions of WordPress. If all the components are "like lodash", i.e., can be shipped in two copies, you can bundle safely. The price you pay is more code shipped to user's browser, but that's the entire price.

Another way to frame the idea is that bundling a package should have predictable consequences. By bundling @wordpress/components, you probably expected that WC Admin starts shipping a bit of bloat, but didn't expect the app to break.

Bundling @wordpress/hooks and @wordpress/slot-fill has very different expectations and consequences, so they should live separately from @wordpress/components.

[gziolo]

I forgot to comment on this issue. Thank you for opening this discussion and sharing your perspective which I can totally relate to.

There are a few issues with @wordpress/components introduced long before we had monorepo and it's one of them. I completely agree that SlotFill deserves its own package that exposes universal API. The current architecture has several flaws as pointed out and it would be the best way to address them. As usual the challenge is to keep this backward compatibility layer but I'm sure it's doable even if we were to duplicate most of the code.

It's also worth mentioning that internally there are two different implementation of SlotFill. @diegohaz and @youknowriad should have the best advices which API to extract and promote moving forward.

We can also discuss technical aspects like hiding or getting rid completely of strings used for SlotFill in the exploratory PRs. For backward compatibility we can always create mapping layer. The truth is that those strings only create issues 😃

[jsnajdr]

I did the slot-fill extraction in #27462, at this time without any API changes. Merely moving the sources to another package, adding backward-compatible reexports to @wordpress/components, and updating all usages in Gutenberg monorepo to use the new package directly.

This should make @wordpress/components more safe to bundle. Not 100%, because there's at least one other singleton component: DropZoneProvider and DropZone. Each DropZone registers itself in a Set inside the provider on mount. If there is no matching provider mounted, the code will probably crash on dropZones.add(), because the dropZones context value is undefined rather than a valid Set.

[youknowriad]

The ultimate framework-level solution would be to remove that singleton, and make @wordpress/components export only the createSlotFill function, not an instance of the context. Each createSlotFill would create its own context (I think it would be even a performance improvement), managed by the specific package rather than by @wordpress/components library.

This is actually something I already built in an old PR for another reason but closed it because I didn't feel that there were big benefits.

Moving to another package

I'm not sure that's a great idea tbh. Just more deprecations and still requires backward compatibility which means the components package still holds a singleton.

---

I personally wonder if the main issue here is that a WP plugin is bundling something that already exists in WPAdmin. So I'm not sure we should make changes in the package itself.

[diegohaz]

@diegohaz and @youknowriad should have the best advices which API to extract and promote moving forward.

I think the bubblesVirtually approach (that uses React Portal) is the one we agreed to stick with. But I'm not sure we can get rid of the non-portal slot-fill from the codebase (and not only for backward compatibility reasons) because React Native doesn't support portals and we still don't have an alternative to this?

Or maybe it's possible to implement slot/fill without a context, making the slot and the fills communicate in some different, more direct way?

I've thought about using the DOM as the source of truth in this case. Instead of using string keys and context, we would pass DOM elements or selectors to Fills, and fillProps would be attached to the DOM nodes. This is very hacky and non-reacty though.

Returning separate contexts from createSlotFill sounds like a better idea.

[gziolo]

But I'm not sure we can get rid of the non-portal slot-fill from the codebase (and not only for backward compatibility reasons) because React Native doesn't support portals and we still don't have an alternative to this?

There is always a middle ground solution for React Native, we move the non-portal implementation to .native.js file to make it fit best to what platforms can offer.

[jsnajdr]

Just more deprecations and still requires backward compatibility which means the components package still holds a singleton.

After this change, the components package no longer holds the singleton -- it just provides access to it. Even if the components package is bundled, like in WC Admin, an import { Slot } from '@wordpress/components' translates into an externalized wp.slotFill.Slot reference. Not to a duplicate instance of the SlotFillContext, as it was until now.

We don't need to deprecate the @wordpress/components exports as they continue to be OK. Just like we didn't deprecate the primitives reexports in components.

I personally wonder if the main issue here is that a WP plugin is bundling something that already exists in WPAdmin. So I'm not sure we should make changes in the package itself.

There's also the case where someone uses the NPM-published packages to build their own editor, and needs to manage the possible duplication of @wordpress/* packages inside their node_modules folder. Here the @wordpress/slot-fill creation is relevant to the peer dependencies work in #26954. @wordpress/slot-fill should be declared as a peer dependency of its consumers, because it contains a singleton and duplication breaks the app. And @wordpress/components gets closer to being a valid normal dependency. I.e., it can be duplicated, and when duplicated, the consequence is only code bloat and not a broken app.

[jordesign]

@jsnajdr Just checking back through some older issues - I just wanted to check if this is an ongoing concern, or if it has been resolved otherwise?

[jsnajdr]

Yes, I think it's still a good idea that slot-fill would be a standalone package. The deduplication problems still exist. But it's rather low priority.