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

Feat: async load config esm #35

Merged
merged 8 commits into from
Apr 27, 2021
Merged

Feat: async load config esm #35

merged 8 commits into from
Apr 27, 2021

Conversation

dominikg
Copy link
Member

@dominikg
Copy link
Member Author

for some reason running the autoprefixer-browserslist tests with jest fails to read svelte config. it works when starting it manually. :/ Investigation tomorrow. If you read this and have ideas whats happening, i'd love to hear more (;

@dummdidumm
Copy link
Member

No idea honestly, maybe Jest isn't that ESM-dynamic-imports-ready either, yet?
Btw I think it makes sense to add another test with a ESM-style config.

@dominikg
Copy link
Member Author

@dummdidumm added fallback to require, custom configFile option and a playground setup for good measure.
Unfortunately the esm config cannot be loaded by jest but i validated that it starts manually.

Would be nice if you had a quick look anyways https://github.com/sveltejs/vite-plugin-svelte/pull/35/files#diff-5fec1310cbc9274dbd46e92e8e144abbc89c90f4b8b103ef5f37f4fe6e6f15a1R13-R62

@dummdidumm
Copy link
Member

Looks good to me 👍 one more thing: you might want to add .mjs to the list of "try to load it via import()", which would enable people to use a esm config file while not setting "type": "module" in their package JSON (might be the case in non-SvelteKit scenarios)

@dominikg dominikg merged commit 4018ce6 into main Apr 27, 2021
@dominikg dominikg deleted the feat/async-load-config-esm branch April 27, 2021 18:42
@github-actions github-actions bot mentioned this pull request Apr 27, 2021
@github-actions github-actions bot mentioned this pull request Jul 13, 2022
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.

2 participants