Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add orderPreprocessors option #86

Closed
wants to merge 0 commits into from
Closed

Conversation

matyunya
Copy link

This addresses the specific case when Sass preprocessor needs to be applied before Svelte Image (style before markup), because Svelte parser throws on encountering sass syntax in the style section.

@atwoodhouse
Copy link

The option orderPreprocessors should probably be renamed since it's a bit counter-intuitive. What it does is that it tells rollup-plugin-svelte NOT to reorder the preprocessors but apply them in exactly the order we passed.

Maybe strictOrder: true or reorder: false (with true being the default) would be clearer.

Apart from the renaming I really vote for this being merged into rollup-plugin-svelte since it's critical to be able to apply style preprocessors before markup preprocessors in some cases such as this: https://github.com/atwoodhouse/example-svelte-multiple-preprocessors/
For reference, the discussion leading up to this PR: sveltejs/svelte#4141

@matyunya
Copy link
Author

I think strictOrder sounds better, renamed the option.

@Conduitry
Copy link
Member

Conduitry commented Dec 31, 2019

I think I would prefer to have this option added to the underlying svelte.preprocess() function, so that it could be used in any build system. We'd still need to update this plugin to pass along the value of the option, but none of the rest of the work here seems to be specific to Rollup.

It also doesn't look like this code would populate the dependencies array when strictOrder is enabled.

@matyunya
Copy link
Author

matyunya commented Jan 7, 2020

Here's the PR: sveltejs/svelte#4222.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants