fix(dev-server-rollup): add basic support for transform objects

[akrawitz]

The transform hook in a Rollup plugin can be either a function or an object with a handler function: https://rollupjs.org/plugin-development/#build-hooks

If transform is not a function, this adds support for a transform object with a handler function, instead of silently failing.

Fixes #3120

[changeset-bot]

🦋 Changeset detected

Latest commit: bce045b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@web/dev-server-rollup	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[bashmish]

IIUC the main reason to define transform function in an object with a handler key is to also define order, see order

e.g.

{
		name: 'transform-plugin',
		transform: {
			order: 'pre',
			handler(code, id) {
				// do transform
			}
		}
	};

but I don't think we currently handle order in the adapter
in other words, if you add support for transform.handler, then it should also implement the ordering, otherwise it doesn't seem to solve any real-world use-case and instead might actually introduce bugs

and also I think all hooks support .handler and .order, so might be good to check if for other hooks it's also needed

UPDATE

in the original issus report you write

For example, @rollup/plugin-yaml has moved from a transform function to an object with a handler function in version 5.0.0:
https://github.com/rollup/plugins/blob/639f45638234c1c3fabfb13615c78bebaef89ef2/packages/yaml/src/index.js#L32

the linked code shows also the usage of .filter, which is also not supported in this PR, and the lack of .filter support might as well break the yaml plugin, right?

[akrawitz]

I agree that this is adding only the most minimal support for transform.handler. As you point out, it does not deal with transform.order or transform.filter (or transform.sequential).

However, the real world use case is to be able to continue using a plugin that has moved from a transform function to a transform object in the same way that I was using it before. In other words, not relying on any new functionality provided by order, filter, or sequential.

I agree that this partial support is less than ideal, but the fact is that @web/dev-server-rollup is only providing partial support for Rollup plugins more generally. For example, only a subset of hooks are supported (see https://modern-web.dev/docs/dev-server/plugins/rollup/#compatibility-with-rollup-plugins as compared to https://rollupjs.org/plugin-development/#build-hooks).

I would also note that (as far as I can tell) the code dealing with the idResolve hook is already taking this approach of "cherrypicking" idResolve.handler while ignoring order, filter, and sequential:

web/packages/dev-server-rollup/src/rollupAdapter.ts

Lines 213 to 214 in b949cff

	const idResolverHandler =
	typeof idResolver === 'function' ? idResolver : idResolver.handler;

Also, due to the way that filter is getting used in the handler by the YAML plugin, it works just fine.

If it would be helpful, I could try to add a test case using @rollup/plugin-yaml to confirm it works in this sort of basic usage. (Although I haven't even been able to get the existing tests to run successfully, so I might need some help with this...) And I could add a warning to the documentation in the section Compatibility with rollup plugins indicating that order, filter, and sequential are not supported.

Having said all of that, I get that this sort of "partial support" can be confusing for users, so I would understand if you don't want to go down this path.