Proposal: add preprocessor nesting

[happycollision]

This allows for developers to ensure that certain markup functions
are run after other script or style functions have run.

I will take care of further addition of tests and whatnot if there is agreement that this is a feature worth having. I couldn't resist making the tests pass. 😅

Problem

Some preprocessors need to run against the markup function to be useful. In the particular case of Svelte Image, it uses the markup function to gain access to the whole component, then uses svelte.parse to get an AST.

But whoops! If you like developing with Typescript, Svelte Image won't work for you because Typescript isn't removed until the script phase of the preprocessor function, and svelte.parse doesn't understand Typescript.

Workaround

Preprocessors that really need access to markup after all other processors have finished could be forced last in this way (again with svelte image as the example)

const svelte = require("svelte/compiler")
const sveltePreprocess = require("svelte-preprocess")
const image = require("svelte-image")

// Ask developers to define this function or something like it:
function runOthersBeforeImage(otherProcessors) {
  return {
    markup: async ({ content, filename }) => {
      const otherProcessorsReturn = await svelte.preprocess(
        content,
        otherProcessors,
        { filename },
      )
      content = otherProcessorsReturn.code

      const { code } = await image({}).markup({ content, filename })
      return {
        ...otherProcessorsReturn,
        code,
      }
    },
  }
}

module.exports = {
  preprocess: [
    runOthersBeforeImage(
      sveltePreprocess({
        defaults: {
          style: "postcss",
        },
        postcss: true,
      }),
    ),
  ],
  // ... other stuff
}

So this solution works, but wouldn't we be calling svelte.preprocess more than necessary, ultimately? not sure if that has any downsides. Like maybe you lose all sense of "who did what" when it comes to dependencies and source maps. But I could be wrong. I don't know a whole lot about the internals of Svelte, but I do know that if we were to nest everything, all the processing would be done against one instance of PreprocessResult, instead of multiple ones.

Perhaps if my proposal isn't up to snuff, we should at least consider adding documentation that points people toward the workaround above, since this issue comes up from time to time.

My proposal: allow nested preprocessors

Here is my addition to the documentation:

An additional level of nesting allows more control over the timing of subsequent markup functions.

const svelte = require('svelte/compiler');

const { code } = await svelte.preprocess(source, [
	[
		{
			markup: () => {
				console.log('this runs first');
			},
			script: () => {
				console.log('this runs third');
			},
			style: () => {
				console.log('this runs fifth');
			}
		},
		{
			markup: () => {
				console.log('this runs second');
			},
			script: () => {
				console.log('this runs fourth');
			},
			style: () => {
				console.log('this runs sixth');
			}
		}
	],
	{
		markup: () => {
			console.log('this runs seventh');
		},
		script: () => {
			console.log('this runs eighth');
		},
		style: () => {
			console.log('this runs ninth');
		}
	}
], {
	filename: 'App.svelte'
});

My main goal here is to get this pushed all the way into the config files for Svelte, Sapper, and SvelteKit projects. I am guessing that this is either the exact way to do it, or that it is the start of that goal.

---

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs

The need was discussed, if not this implementation, in #5293 and #4141. There is another PR (#4282) that addresses the need, but in a different way. I prefer my method, as the one in the other PR is an all-or-nothing proposition based on an option flag. With this one, you can completely preserve the notion of "all markup, then scripts, then styles" within whatever groups you define.

There are likely other issues that bring up the need to control when preprocessors run compared to others, but I don't know what other wording to search for.

• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with npm test and lint the project with npm run lint

[mcmxcdev]

Awesome that this issue gets tackled, really important!

There is already an open PR from svelte-image core maintainer too: #4282
And my initial reported issue here: #5293

[dummdidumm]

I'm a bit unsure if this is a good enhancement. The current API obviously has these ordering drawbacks. My envisioned solution to this is to just ditch the script and style preprocessors and instead require all preprocessors to preprocess on the whole code, like the current markup preprocessor. That and providing utility-functions to extract the script/style tags with attributes should give the same power with more ordering flexibility.
Obviously this is a breaking change, but I'm not sure if we should make the API more complex in the meantime.

[happycollision]

I am 100 percent in favor of a breaking change that would make this processor ordering more usable and have a simple api.

However, this suggestion is in lieu of something that isn’t backwards compatible.

I suppose another alternative would be a flag to indicate you are using a new version of the config.

In the meantime, I’ll keep this open.

[dummdidumm]

I don't fully understand your answer. Are you in favor of my envisioned proposal and want it sooner rather than later through a config setting (we agree that we can't just change it, because it's a breaking change)?

[happycollision]

I am in favor of either

1. A backwards compatible—though possibly more complex—solution now, while we wait for the simpler, backwards incompatible solution to be used at the next major version.

2. A way to opt-in to the new version you've described via a flag, setting, or different property altogether (preprocessExperimental, etc). If it was documented as experimental, you could get feedback and tweak the new behavior/api until it fit the widest set of needs. At that point it could just wait until v4.0 to become the only way to do preprocessors.

Option 2 seems the better solution to me.

However, I am completely ignorant of how this kind of thing might affect the Language Services stuff.

[dummdidumm]

Right now svelte.preprocess is completely decoupled from svelte.compile. svelte.preprocess expects either and object with one of markup/style/script, or an array of these objects. We could change the code in svelte.preprocess to also support receiving a plain function, which signals that the new experimental version should be used. That would make this experimental change completely transparent for language tools and the rollup/webpack/vite plugin. If we did that, I'm not sure about interop with preprocessors written the "old" way. Throw an error, but also provide a transformation function? Support both modes in parallel?

[happycollision]

As long as the only action required is on the part of the developer of an app, not the developer of a preprocessor. I am guessing that many of those projects will be slow to transition, if ever. So if we provide access to utility functions as you describe that can be used to provide the old api to the preprocessors, I think that's a decent solution.

Something like this?

// somewhere in "svelte/compiler"

export function getScript(content) { /* details */ }
export function getStyle(content) { /* details */ }
export function mergeParts(body, script, style) { /* details */ }

export function legacyPreprocessor(preprocessor) {
  return {
    markup: async ({ content, filename }) => {
      const processedBody =
        await (
          preprocessor?.markup({ content, filename }) ??
          Promise.resolve({ code: content })
        );
      
      const script = getScript(processedBody.code);
      const processedScript =
        await (
          preprocessor?.script({ content: script, filename }) ??
          Promise.resolve({ code: script })
        );
      
      const style = getStyle(processedBody.code);
      const processedStyle =
        await (
          preprocessor?.style({ content: style, filename }) ??
          Promise.resolve({ code: style })
        );
      
      return mergeParts(processedBody, processedScript, processedStyle);
    },
  };
}

// in the config file (or similar depending on your project)

import { legacyPreprocessor } from "svelte/compiler";
import sveltePreprocess from "svelte-preprocess"
import svelteImage from "svelte-image"

const someConfig = {}
const otherConfig = {}

module.exports = {
  // Passing a function to `preprocess` signals only markup will be provided,
  // and everything will execute in order, top to bottom.
  preprocess: () => [
    legacyPreprocessor(sveltePreprocess(someConfig)),
    svelteImage(otherConfig),
  ],
};

Is this getting into RFC territory? I'd love to solve this problem once and for all.

[dummdidumm]

A RFC would be great for this I think. Would you be willing to write one based on your previous observations and this discussion?

[happycollision]

I could, but don’t know how soon I could do it. Do you know the original reason for the different ordering of preprocessing? Seems like a very specific choice. I’ve just never known the upside to doing it that way. I wouldn’t want to write up an rfc if there is some very good reason to keep the ordering as is

[dummdidumm]

Closing in favor of sveltejs/rfcs#56