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

Sync preprocess #5770

Closed
wants to merge 6 commits into from
Closed

Conversation

ehrencrona
Copy link
Contributor

@ehrencrona ehrencrona commented Dec 11, 2020

This is a proposal for allowing synchronous preprocessing in order to solve sveltejs/kit#19 . It allows preprocessors to export synchronous versions of their processing functions. This allows the preprocessor to be used with e.g. require.extensions which is synchronous.

Obviously, not all preprocessors will be able to work synchronously, but most important case is TypeScript, which is.

This PR is based on #5763 ; diffing against it will make it easier to see what the changes to support async are. I needed to do quite extensive refactoring to pool the code that needs to be either sync or async in as few places as possible.

It's still a draft; tests are missing and the code could be refactored further. I'd like feedback as to whether this is a good idea before proceeding.

It goes together with sveltejs/svelte-preprocess#291 . See lukeed/uvu#85 for how it could be used.

@Rich-Harris
Copy link
Member

As mentioned on the SvelteKit repo, I'm 👎 on this — if preprocess is incompatible with require.extensions, I think it's a sign that we shouldn't be encouraging people to use require.extensions, which is a deprecated API that relates to a dying format. The future is ESM with loader hooks (which can be asynchronous); the present is using our existing build pipelines.

@aewing
Copy link

aewing commented Dec 13, 2020

I think synchronous preprocessing would be beneficial for ESLint support as well. sveltejs/eslint-plugin-svelte3#10 (comment)
(Though if that's still true or for how long I am not sure: eslint/rfcs#4)

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