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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "include" option, rename "compileOptions" #86

Merged
merged 1 commit into from
Dec 5, 2021

Conversation

tyler-boyd
Copy link
Contributor

Hey @EMH333, just opening this as a draft for now to get your feedback. Plz disregard the dist/ and gitignore changes; if you're interested in this PR I'll fix it up and remove them.

  • Adds an include option, similar to the svelte rollup plugin's include option. This is necessary to support mixing/matching webcomponents. In general, we include the plugin twice. Once with compilerOptions.customElement = true and include: /\.wc\.svelte$/, and a second time without customElement and with exclude: /\.wc\.svelte$/. I couldn't figure out how to make exclude work so I only did include, and it works well enough for our production build 馃檪
  • Renamed compileOptions to compilerOptions -- small thing, but this lets you import your svelte.config.js and just use that object instead of having to copy in both the compilerOptions and preprocess settings

Let me know what you think, thanks!

@tyler-boyd tyler-boyd changed the title Add "include" option, rename "compileOptions" [DRAFT] Add "include" option, rename "compileOptions" Nov 17, 2021
@EMH333
Copy link
Owner

EMH333 commented Nov 17, 2021

Hey @EMH333, just opening this as a draft for now to get your feedback. Plz disregard the dist/ and gitignore changes; if you're interested in this PR I'll fix it up and remove them.

Thanks for your PR! Definitely interested

Adds an include option, similar to the svelte rollup plugin's include option. This is necessary to support mixing/matching webcomponents. In general, we include the plugin twice. Once with compilerOptions.customElement = true and include: /\.wc\.svelte$/, and a second time without customElement and with exclude: /\.wc\.svelte$/. I couldn't figure out how to make exclude work so I only did include, and it works well enough for our production build 馃檪

Looks good

Renamed compileOptions to compilerOptions -- small thing, but this lets you import your svelte.config.js and just use that object instead of having to copy in both the compilerOptions and preprocess settings

I'd be inclined to also keep compileOptions around for at least another "major" version (this PR, and a few other things I'm working on, would kick us up to 0.6.0 and I'd want to allow but deprecate the old name till we get to 0.7.0). This esbuild plugin tends to be a keystone of people's build code and I try to be conservative when making changes like this. Clear warnings are better than errors when upgrading. a24ef7c is where I renamed preprocessor to preprocess, also in a quest for compatibility. That seemed to work fairly well at the time, but I'm open to hearing your thoughts

@EMH333 EMH333 added the enhancement New feature or request label Nov 17, 2021
@EMH333
Copy link
Owner

EMH333 commented Nov 29, 2021

@tyler-boyd Any updates here?

@tyler-boyd
Copy link
Contributor Author

Thanks @EMH333 , I've updated the PR to be a bit more ready! Backwards compatible, and rebased/cleaned up history.

Please take a look when you can and let me know if there should be any changes!

@EMH333 EMH333 changed the title [DRAFT] Add "include" option, rename "compileOptions" Add "include" option, rename "compileOptions" Dec 3, 2021
@EMH333
Copy link
Owner

EMH333 commented Dec 3, 2021

Looks great, I'll get this merged this weekend. Thank you!

@EMH333 EMH333 merged commit e132b75 into EMH333:main Dec 5, 2021
@EMH333
Copy link
Owner

EMH333 commented Dec 5, 2021

Released as a part of v0.6.0 馃帀

@tyler-boyd
Copy link
Contributor Author

Thank you very much @EMH333! We're now pointing back at the real esbuild-svelte v0.6.0 instead of the fork 馃檪

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

Successfully merging this pull request may close these issues.

None yet

2 participants