Allow jsxFactory and jsxFragment to be passed in buildOptions for use by esbuild #1838
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/6b2f4nvdq |
|
Normally it's good to start a discussion first before creating a PR, in case the work isn't something that we're interested in merging. In this case, are you coming from #1681 ? I think the latest from that thread was that just changing the pragma wouldn't help here to enable the React 17 JSX transform, since that pragma uses a new function signature. See Evan's answer here: #1681 (reply in thread) If you have a different use-case, please share it! |
|
@FredKSchott i should have included that context, sorry about that. I saw this discussion #1422 in which you mentioned you’d love to see a PR exposing these options via buildOptions. As little background, I’ve been writing my own frontend framework which uses JSX. I wanted to use snowpack to build it, but noticed there was no way to change the JSX factory or fragment functions without using Babel instead of esbuild. I thought it would be an easy win here to support anyone not using React or Preact to allow these options to be passed, since all of the hard work has already been implemented on the esbuild side. The options just needed to be exposed. |
| const isPreact = checkIsPreact(filePath, contents); | ||
| if (isPreact) { | ||
| jsxFactory || (jsxFactory = 'h') |
There was a problem hiding this comment.
This is a bit different from how we treat conditional variable setting in the rest of the codebase. for consistency, please do something closer to this:
const jsxFactory = config.buildOptions.jsxFactory ?? (isPreact ? 'h' : undefined);
const jsxFragment = config.buildOptions. jsxFragment ?? (isPreact ? 'Fragment' : undefined);There was a problem hiding this comment.
Sounds good. The nullish operator is much better here, glad you guys are using them 👍
| @@ -194,6 +194,14 @@ module.exports = { | |||
|
|
|||
| - Rename your web modules directory. | |||
|
|
|||
| #### buildOptions.jsxFactory | `string` | Default: `React.createElement` or `h` if Preact is detected | |||
There was a problem hiding this comment.
nit:
Default:
React.createElement(orh, if a Preact import is detected)
Default:React.Fragment(orFragment, if a Preact import is detected)
snowpack/src/config.ts
Outdated
| @@ -68,6 +68,8 @@ const DEFAULT_CONFIG: SnowpackUserConfig = { | |||
| sourceMaps: false, | |||
| watch: false, | |||
| htmlFragments: false, | |||
| jsxFactory: undefined, | |||
There was a problem hiding this comment.
if undefined, just leave unset in this default object. You'll need to update the SnowpackConfig TypeScript interface as well.
There was a problem hiding this comment.
I added it because I saw hmrPort was set to undefined above in dev options. Makes sense, I'll remove it.
Changes
This PR adds the options jsxFactory and jsxFragment to the buildOptions object within the snowpack config. It allows users to specify a custom jsx factory and fragment function without needing to use babel instead of esbuild. Nothing complex, just passing the options through as esbuild already supports them.
Testing
WIP - This PR is a draft because I wasn't sure how best to create the tests for these options. A recommendation on what kind of test is required from the project's maintainers here is appreciated. I'll update the PR accordingly.
Docs
Added the options to the reference docs for the build options.