Add preact/dom package

[jridgewell]

AMP is attempting to ship npm modules containing both preact and react variants of our components. One of the sticking points was how do we easily remap from our preact imports into the equivalent react import. For createElement, it's easy. But for render and hydrate, it's very difficult to remap from preact to react-dom.

Because preact (or preact/compat) has mixed react and react-dom APIs, we have to inspect both the import source and the specifiers in order to remap. That's not easily supported, and would essentially require us to write a custom Babel plugin to transform.

Instead, I opted to create a placeholder preact/dom package (we patch our node_modules directory during builds). This file just exports the needed functions from preact package. Now, I can easily remap preact/dom to react-dom without inspecting what specifiers are imported.

[coveralls]

Coverage remained the same at 99.445% when pulling 34dd1a897abf03adc97eac3879bfa2c9952cb64c on jridgewell:preact-dom into 4e29ec70332c8c1c7cfb002d4069f2c9db2831f2 on preactjs:master.

[marvinhagemeister]

Thanks for opening a PR! I feel like I don't have a good grasp yet on what the advantages are of having a preact/dom package entry.

Because preact (or preact/compat) has mixed react and react-dom APIs, we have to inspect both the import source and the specifiers in order to remap. That's not easily supported, and would essentially require us to write a custom Babel plugin to transform.

I'm assuming the aliasing itself isn't the problem (react-dom -> preact/dom vs react-dom -> preact/compat), so there seems to be another reason behind it. I've read through the PR on the AMP repo and to me the main difference seems to be moving the toChildArray() export around. We could think about exporting that from preact/compat too.

Can you share more details about the advantages of the preact/dom approach? I'm unsure I grasp the problem it solves so far, and I feel like I'm missing a piece of the puzzle to get it.

[jridgewell]

I'm assuming the aliasing itself isn't the problem (react-dom -> preact/dom vs react-dom -> preact/compat), so there seems to be another reason behind it.

You're thinking about it the wrong way, we're trying map preact -> react.

import { createElement, render } from "preact";

How do we remap this import into the react equivalents? Well we can't, because we can't import from 2 remapped sources with the same import. We first need to split into two (1 import that can be mapped into react, and one that can be mapped to react-dom)

import { createElement } from "preact";
import { render } from "preact";

But, we still have a problem! Remapping for us looks at just the import source. Both sources here are preact, so we still can't figure out what package to remap to. I have to look at the imported specifiers to know how to remap.

The way we're solving this is to create a preact/dom package:

import { createElement } from "preact";
import { render } from "preact/dom";

Now, I can inspect just the import source, and can easily remap preact -> react and preact/dom -> react-dom.

[developit]

@jridgewell This seems reasonable and I understand the logic for splitting things up. However, I'm wondering if it would be simpler to do the reverse, and create a wrapper module around react + react-dom that merges its exports? That wrapper would also be a decent place to explicitly map React's CommonJS interface to proper named exports. Thoughts?

One of the reasons I'm pondering this is that, if we land a preact/dom package in core, it's sortof giving the indication that there's a reason we'd split out that functionality. We don't currently have a reason to pursue that that I know of.

[marvinhagemeister]

@jridgewell Ah that makes sense. I didn't think of that use case 👍

We could also name it preact/compat-dom to make it more clear that it's a compatibility package for React. I'd prefer that over preact/dom in case we want to use the latter some time in the future (there are no current plans for that, but who knows what we'll do in 5 years)

[developit]

Another option: preact/compat/dom (for the package.json resolution).

[jridgewell]

However, I'm wondering if it would be simpler to do the reverse, and create a wrapper module around react + react-dom that merges its exports?

I think that would be a bit weird, since it wouldn't be an official React package? AMP could internally maintain a wrapper, but it'd be essentially dead code in the repo and only used when we remapped.

One of the reasons I'm pondering this is that, if we land a preact/dom package in core, it's sortof giving the indication that there's a reason we'd split out that functionality.

I can understand how someone would assume that, but I don't think it's a requirement. I see it more of a statement of compatibility with React. Where React's official package is react-dom, we'll provide preact/dom to give the same surface area. Where that functionality is actually implemented won't really matter.

We could also name it preact/compat-dom to make it more clear that it's a compatibility package for React

I'd be happy with that, too. If we can get general agreement, I can update the PR.

Another option: preact/compat/dom (for the package.json resolution).

Will nested directories work correctly? We'd need to provide package.json exports at 3 levels in this case, right?

[developit]

@jridgewell re preact/compat/dom - yes, that would require another package.json at compat/dom/package.json and additional exports in the root package.json. From a consumption standpoint, it would be as if there was an npm package called preact/compat/dom, despite it being the preact package and a subdirectory - same premise we use for preact/compat.

Another thing to consider here, which I think weighs in favor of preact/compat/dom, is that we have a wonky preact/compat/scheduler.{js,mjs} alias right now that needs to be moved into a directory with its own package.json and mirrored exports in the root package.json.