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 input option to plugin-svelte #1347

Merged

Conversation

firefish5000
Copy link
Contributor

Changes

Adds an input option to plugin-svelte that modifies resolve.input. Enables non-standard file extensions such as .svx to be used

Testing

Roughly copied the related tests from plugin-babel. Had issues getting test to run properly locally, so opening the PR a bit early to see the results.

Docs

Roughly copied related docs from plugin-babel.

@vercel
Copy link

vercel bot commented Oct 17, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/7xd8fu4k3
✅ Preview: https://snowpack-git-svelte-resolve-input-option.pikapkg.vercel.app

@firefish5000
Copy link
Contributor Author

Test Passed. Ready for review

@canadaduane
Copy link
Contributor

I use svelte, but haven't heard of the svx convention before. Out of curiosity, how common is this?

@firefish5000
Copy link
Contributor Author

firefish5000 commented Oct 18, 2020

@canadaduane This PR adds ability to modify the file extensions we run on, primarily to add things in addition to .svelte. It add feature parity with the rollup and webpack svelte plugins.

Overriding extensions for plain svelte files is not a standard convention, but a couple of popular preprocessors and plugins do this. Namely the markdown preprocessor for svelte, pngwn/MDsveX, which uses .svx as its convention (but can be made to run on normal .svelte files as well)
There is also the storybook replacement, rixo/svench which runs on .svench (and also recommends using mdsvex for documentation).

Its worth noting that for mdsvex, using another file extension is sort of required since it treats the entire file as html enabled markdown without checking/needing a lang="md" attribute. Meaning <p> *words* <p> will italicize, something you probably dont want in a normal svelte file

Copy link
Owner

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@FredKSchott FredKSchott merged commit 8ecc4a1 into FredKSchott:master Oct 18, 2020
@firefish5000
Copy link
Contributor Author

Thinking about why he asked made me realize I put ['.svelte','.svx'] into the recommendation of the error output instead of just the default value ['.svelte'], as I did in the documentation. This was primarily to show it was an array of file extensions and to sort of to match what the babel plugin threw, but I can see how it could be confusing. Probably would have been better to just recommend the default

@FredKSchott
Copy link
Owner

I think it's fine as-is. It would only show that to the user who's playing around with the input setting, so it's not necessarily the "default" user who would see that message.

@canadaduane
Copy link
Contributor

canadaduane commented Oct 18, 2020 via email

@techniq
Copy link
Contributor

techniq commented Oct 20, 2020

Could this be defaulted to the extensions value in svelte.config.js to cut down on duplication?

image

@firefish5000
Copy link
Contributor Author

firefish5000 commented Oct 20, 2020

@techniq While I also use extensions in my svelte.config.js (since that is the name of the rollup plugin's option, and I need to keep my svelte.preprocess cli in sync with my builder/bundler), I believe the field is non-standard and not recommended in the language-tools docs. I am doubtful it will become a standard as well since iirc, webpack uses a different mechanism for matching files with loaders. (meaning rollup plugin is the only thing that has a extensions field, and everything else uses a different name and/or method for matching svelte files)

@firefish5000 firefish5000 deleted the svelte-resolve-input-option branch October 20, 2020 20:03
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.

4 participants