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

fix: Strips jsxFactory from tsconfig if --importSource is set #988

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rschristian
Copy link
Collaborator

Closes #857

If a user supplies --jsxImportSource we should remove jsxFactory and jsxFragmentFactory from the default tsconfig as they're mutually exclusive with jsx: 'react-jsx'.

@changeset-bot
Copy link

changeset-bot bot commented Aug 10, 2022

🦋 Changeset detected

Latest commit: e4d8db3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
microbundle Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@rschristian rschristian changed the title fix: Strips jsxFactory from tsconfig if --imporSource is set fix: Strips jsxFactory from tsconfig if --importSource is set Aug 10, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 10, 2022

Size Change: 0 B

Total Size: 63.4 kB

ℹ️ View Unchanged
Filename Size
test/fixtures/alias-external/dist/alias-external.esm.mjs 85 B
test/fixtures/alias-external/dist/alias-external.js 80 B
test/fixtures/alias-external/dist/alias-external.umd.js 140 B
test/fixtures/alias/dist/alias-mapping.esm.mjs 62 B
test/fixtures/alias/dist/alias-mapping.js 62 B
test/fixtures/alias/dist/alias-mapping.umd.js 118 B
test/fixtures/async-iife-ts/dist/async-iife-ts.esm.mjs 114 B
test/fixtures/async-iife-ts/dist/async-iife-ts.js 111 B
test/fixtures/async-iife-ts/dist/async-iife-ts.umd.js 161 B
test/fixtures/async-iife-ts/dist/index.d.ts 20 B
test/fixtures/async-ts/dist/async-ts.esm.mjs 159 B
test/fixtures/async-ts/dist/async-ts.js 149 B
test/fixtures/async-ts/dist/async-ts.umd.js 247 B
test/fixtures/async-ts/dist/index.d.ts 75 B
test/fixtures/basic-babelrc/dist/basic-babelrc.esm.mjs 160 B
test/fixtures/basic-babelrc/dist/basic-babelrc.js 157 B
test/fixtures/basic-babelrc/dist/basic-babelrc.umd.js 198 B
test/fixtures/basic-compress-false/dist/basic-compress-false.esm.mjs 302 B
test/fixtures/basic-compress-false/dist/basic-compress-false.js 300 B
test/fixtures/basic-compress-false/dist/basic-compress-false.umd.js 422 B
test/fixtures/basic-css/dist/basic-css.css 106 B
test/fixtures/basic-css/dist/basic-css.esm.mjs 143 B
test/fixtures/basic-css/dist/basic-css.js 139 B
test/fixtures/basic-css/dist/basic-css.umd.js 227 B
test/fixtures/basic-dashed-external/dist/basic-dashed-external.esm.mjs 256 B
test/fixtures/basic-dashed-external/dist/basic-dashed-external.js 312 B
test/fixtures/basic-dashed-external/dist/basic-dashed-external.umd.js 395 B
test/fixtures/basic-flow/dist/basic-lib-flow.esm.mjs 104 B
test/fixtures/basic-flow/dist/basic-lib-flow.js 100 B
test/fixtures/basic-flow/dist/basic-lib-flow.umd.js 206 B
test/fixtures/basic-json/dist/basic-json.esm.mjs 128 B
test/fixtures/basic-json/dist/basic-json.js 125 B
test/fixtures/basic-json/dist/basic-json.umd.js 217 B
test/fixtures/basic-multi-source-css/dist/a.css 97 B
test/fixtures/basic-multi-source-css/dist/a.esm.mjs 135 B
test/fixtures/basic-multi-source-css/dist/a.js 130 B
test/fixtures/basic-multi-source-css/dist/a.umd.js 226 B
test/fixtures/basic-multi-source-css/dist/b.css 96 B
test/fixtures/basic-multi-source-css/dist/b.esm.mjs 135 B
test/fixtures/basic-multi-source-css/dist/b.js 131 B
test/fixtures/basic-multi-source-css/dist/b.umd.js 227 B
test/fixtures/basic-multi-source/dist/a.esm.mjs 78 B
test/fixtures/basic-multi-source/dist/a.js 73 B
test/fixtures/basic-multi-source/dist/a.umd.js 133 B
test/fixtures/basic-multi-source/dist/b.esm.mjs 78 B
test/fixtures/basic-multi-source/dist/b.js 73 B
test/fixtures/basic-multi-source/dist/b.umd.js 133 B
test/fixtures/basic-no-compress/dist/basic-no-compress.esm.mjs 300 B
test/fixtures/basic-no-compress/dist/basic-no-compress.js 298 B
test/fixtures/basic-no-compress/dist/basic-no-compress.umd.js 422 B
test/fixtures/basic-no-pkg-main/dist/basic-no-pkg-main.js 315 B
test/fixtures/basic-node-internals/dist/basic-node-internals.js 232 B
test/fixtures/basic-ts/dist/basic-lib-ts.esm.mjs 156 B
test/fixtures/basic-ts/dist/basic-lib-ts.js 153 B
test/fixtures/basic-ts/dist/basic-lib-ts.umd.js 235 B
test/fixtures/basic-ts/dist/car.d.ts 117 B
test/fixtures/basic-ts/dist/index.d.ts 83 B
test/fixtures/basic-tsx/dist/basic-lib-tsx.esm.mjs 254 B
test/fixtures/basic-tsx/dist/basic-lib-tsx.js 246 B
test/fixtures/basic-tsx/dist/basic-lib-tsx.umd.js 331 B
test/fixtures/basic-tsx/dist/index.d.ts 69 B
test/fixtures/basic-with-cwd/basic/dist/basic.esm.mjs 224 B
test/fixtures/basic-with-cwd/basic/dist/basic.js 221 B
test/fixtures/basic-with-cwd/basic/dist/basic.umd.js 301 B
test/fixtures/basic/dist/basic-lib.esm.mjs 227 B
test/fixtures/basic/dist/basic-lib.js 224 B
test/fixtures/basic/dist/basic-lib.umd.js 307 B
test/fixtures/class-decorators-ts/dist/class-decorators-ts.esm.mjs 388 B
test/fixtures/class-decorators-ts/dist/class-decorators-ts.js 386 B
test/fixtures/class-decorators-ts/dist/class-decorators-ts.umd.js 451 B
test/fixtures/class-decorators-ts/dist/index.d.ts 123 B
test/fixtures/class-properties/dist/class-properties.esm.mjs 140 B
test/fixtures/class-properties/dist/class-properties.js 135 B
test/fixtures/class-properties/dist/class-properties.umd.js 235 B
test/fixtures/css-modules--false/dist/css-modules--false.css 114 B
test/fixtures/css-modules--false/dist/css-modules--false.esm.mjs 52 B
test/fixtures/css-modules--false/dist/css-modules--false.js 49 B
test/fixtures/css-modules--false/dist/css-modules--false.umd.js 156 B
test/fixtures/css-modules--null/dist/css-modules--null.css 118 B
test/fixtures/css-modules--null/dist/css-modules--null.esm.mjs 112 B
test/fixtures/css-modules--null/dist/css-modules--null.js 110 B
test/fixtures/css-modules--null/dist/css-modules--null.umd.js 200 B
test/fixtures/css-modules--string/dist/css-modules--string.css 144 B
test/fixtures/css-modules--string/dist/css-modules--string.esm.mjs 168 B
test/fixtures/css-modules--string/dist/css-modules--string.js 166 B
test/fixtures/css-modules--string/dist/css-modules--string.umd.js 263 B
test/fixtures/css-modules--true/dist/css-modules--true.css 115 B
test/fixtures/css-modules--true/dist/css-modules--true.esm.mjs 141 B
test/fixtures/css-modules--true/dist/css-modules--true.js 139 B
test/fixtures/css-modules--true/dist/css-modules--true.umd.js 230 B
test/fixtures/custom-babelrc/dist/custom-babelrc.esm.mjs 259 B
test/fixtures/custom-babelrc/dist/custom-babelrc.js 252 B
test/fixtures/custom-babelrc/dist/custom-babelrc.umd.js 343 B
test/fixtures/custom-source-with-cwd/custom-source/dist/custom-src.esm.mjs 227 B
test/fixtures/custom-source-with-cwd/custom-source/dist/custom-src.js 224 B
test/fixtures/custom-source-with-cwd/custom-source/dist/custom-src.umd.js 306 B
test/fixtures/custom-source/dist/custom-source.esm.mjs 227 B
test/fixtures/custom-source/dist/custom-source.js 224 B
test/fixtures/custom-source/dist/custom-source.umd.js 307 B
test/fixtures/default-named/dist/default-named.esm.mjs 116 B
test/fixtures/default-named/dist/default-named.js 97 B
test/fixtures/default-named/dist/default-named.umd.js 201 B
test/fixtures/define-expression/dist/define-expression.esm.mjs 56 B
test/fixtures/define/dist/define.esm.mjs 55 B
test/fixtures/define/dist/define.js 55 B
test/fixtures/define/dist/define.umd.js 108 B
test/fixtures/esnext-ts/dist/esnext-ts.esm.mjs 1.05 kB
test/fixtures/esnext-ts/dist/esnext-ts.js 1.04 kB
test/fixtures/esnext-ts/dist/esnext-ts.umd.js 1.11 kB
test/fixtures/esnext-ts/dist/index.d.ts 68 B
test/fixtures/inline-source-map/dist/inline-source-map.esm.mjs 740 B
test/fixtures/inline-source-map/dist/inline-source-map.js 739 B
test/fixtures/inline-source-map/dist/inline-source-map.umd.js 827 B
test/fixtures/jsx/dist/jsx.esm.mjs 267 B
test/fixtures/jsx/dist/jsx.js 264 B
test/fixtures/jsx/dist/jsx.umd.js 341 B
test/fixtures/macro/dist/macro-lib.esm.mjs 88 B
test/fixtures/macro/dist/macro-lib.js 84 B
test/fixtures/macro/dist/macro-lib.umd.js 187 B
test/fixtures/mangle-json-file/dist/mangle-json-file.esm.mjs 148 B
test/fixtures/mangle-json-file/dist/mangle-json-file.js 143 B
test/fixtures/mangle-json-file/dist/mangle-json-file.umd.js 235 B
test/fixtures/minify-config-boolean/dist/minify-config-boolean.esm.mjs 161 B
test/fixtures/minify-config-boolean/dist/minify-config-boolean.js 152 B
test/fixtures/minify-config-boolean/dist/minify-config-boolean.umd.js 254 B
test/fixtures/minify-config/dist/minify-config.esm.mjs 143 B
test/fixtures/minify-config/dist/minify-config.js 137 B
test/fixtures/minify-config/dist/minify-config.umd.js 226 B
test/fixtures/minify-path-config/dist/minify-path-config.esm.mjs 151 B
test/fixtures/minify-path-config/dist/minify-path-config.js 145 B
test/fixtures/minify-path-config/dist/minify-path-config.umd.js 237 B
test/fixtures/minify-path-parent-dir-with-cwd/minify-path-parent-dir/dist/minify-path-parent-dir.esm.mjs 153 B
test/fixtures/minify-path-parent-dir-with-cwd/minify-path-parent-dir/dist/minify-path-parent-dir.js 149 B
test/fixtures/minify-path-parent-dir-with-cwd/minify-path-parent-dir/dist/minify-path-parent-dir.umd.js 242 B
test/fixtures/modern-generators/dist/modern-generators.esm.mjs 282 B
test/fixtures/modern-generators/dist/modern-generators.js 279 B
test/fixtures/modern-generators/dist/modern-generators.modern.mjs 159 B
test/fixtures/modern-generators/dist/modern-generators.umd.js 358 B
test/fixtures/modern/dist/modern-lib.modern.mjs 152 B
test/fixtures/name-custom-amd/dist/name-custom-amd.esm.mjs 230 B
test/fixtures/name-custom-amd/dist/name-custom-amd.js 226 B
test/fixtures/name-custom-amd/dist/name-custom-amd.umd.js 312 B
test/fixtures/name-custom-cli/dist/name-custom.esm.mjs 227 B
test/fixtures/name-custom-cli/dist/name-custom.js 224 B
test/fixtures/name-custom-cli/dist/name-custom.umd.js 311 B
test/fixtures/no-pkg-name/dist/no-pkg-name.esm.mjs 229 B
test/fixtures/no-pkg-name/dist/no-pkg-name.js 225 B
test/fixtures/no-pkg-name/dist/no-pkg-name.umd.js 311 B
test/fixtures/no-pkg/dist/no-pkg.esm.mjs 225 B
test/fixtures/no-pkg/dist/no-pkg.js 222 B
test/fixtures/no-pkg/dist/no-pkg.umd.js 303 B
test/fixtures/optional-chaining-ts/dist/index.d.ts 107 B
test/fixtures/optional-chaining-ts/dist/optional-chaining-ts.esm.mjs 154 B
test/fixtures/optional-chaining-ts/dist/optional-chaining-ts.js 148 B
test/fixtures/optional-chaining-ts/dist/optional-chaining-ts.umd.js 247 B
test/fixtures/parameters-rest-closure/dist/parameters-rest-closure.esm.mjs 202 B
test/fixtures/parameters-rest-closure/dist/parameters-rest-closure.js 191 B
test/fixtures/parameters-rest-closure/dist/parameters-rest-closure.umd.js 283 B
test/fixtures/pretty/dist/pretty.esm.mjs 6.39 kB
test/fixtures/pretty/dist/pretty.js 6.37 kB
test/fixtures/pretty/dist/pretty.umd.js 6.5 kB
test/fixtures/publish-config/dist/bar.js 87 B
test/fixtures/publish-config/dist/foo.d.ts 60 B
test/fixtures/pure/dist/pure.esm.mjs 108 B
test/fixtures/pure/dist/pure.js 98 B
test/fixtures/pure/dist/pure.umd.js 197 B
test/fixtures/raw/dist/raw.esm.mjs 1.43 kB
test/fixtures/raw/dist/raw.js 1.42 kB
test/fixtures/raw/dist/raw.umd.js 1.53 kB
test/fixtures/shebang/dist/shebang.esm.mjs 125 B
test/fixtures/shebang/dist/shebang.js 118 B
test/fixtures/shebang/dist/shebang.umd.js 216 B
test/fixtures/terser-annotations/dist/terser-annotations.esm.mjs 179 B
test/fixtures/terser-annotations/dist/terser-annotations.js 171 B
test/fixtures/terser-annotations/dist/terser-annotations.umd.js 258 B
test/fixtures/ts-custom-declaration/dist/index.esm.mjs 100 B
test/fixtures/ts-custom-declaration/dist/index.js 89 B
test/fixtures/ts-custom-declaration/dist/index.umd.js 206 B
test/fixtures/ts-declaration/dist/index.esm.mjs 100 B
test/fixtures/ts-declaration/dist/index.js 89 B
test/fixtures/ts-declaration/dist/index.umd.js 202 B
test/fixtures/ts-jsx/dist/index.d.ts 52 B
test/fixtures/ts-jsx/dist/ts-jsx.esm.mjs 164 B
test/fixtures/ts-jsx/dist/ts-jsx.js 159 B
test/fixtures/ts-jsx/dist/ts-jsx.umd.js 252 B
test/fixtures/ts-mixed-exports/dist/car.d.ts 117 B
test/fixtures/ts-mixed-exports/dist/index.d.ts 89 B
test/fixtures/ts-mixed-exports/dist/ts-mixed-exports.esm.mjs 168 B
test/fixtures/ts-mixed-exports/dist/ts-mixed-exports.js 159 B
test/fixtures/ts-mixed-exports/dist/ts-mixed-exports.umd.js 261 B
test/fixtures/ts-module/dist/foo.d.ts 60 B
test/fixtures/ts-module/dist/index.d.ts 63 B
test/fixtures/ts-module/dist/ts-module.esm.mjs 108 B
test/fixtures/ts-module/dist/ts-module.js 97 B
test/fixtures/ts-module/dist/ts-module.umd.js 197 B
test/fixtures/visualizer/dist/visualizer.esm.mjs 119 B
test/fixtures/visualizer/dist/visualizer.js 177 B
test/fixtures/visualizer/dist/visualizer.umd.js 260 B
test/fixtures/worker-loader/dist/worker-35b22e56.js 125 B
test/fixtures/worker-loader/dist/worker-7e1b9921.js 107 B
test/fixtures/worker-loader/dist/worker-loader.esm.mjs 180 B
test/fixtures/worker-loader/dist/worker-loader.modern.mjs 174 B

compressed-size-action

@rschristian rschristian force-pushed the fix/jsx-import-source-tsconfig branch from 2f6f13f to 74de373 Compare August 10, 2022 07:37
@rschristian rschristian force-pushed the fix/jsx-import-source-tsconfig branch from 74de373 to e4d8db3 Compare August 10, 2022 19:32
Comment on lines +553 to +562
...(options.jsxImportSource
? {
jsx: 'react-jsx',
jsxImportSource: options.jsxImportSource,
}
: {
jsx: 'preserve',
jsxFactory: options.jsx,
jsxFragmentFactory: options.jsxFragment,
}),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Theoretically we would be fine to leave jsx untouched & remove that first piece of the ternary, however, that relies on the assumption that the user does have a tsconfig.json and that it's correctly set.

Conditionally adding jsxFactory (and jsxFragmentFactory) is enough to quell the errors we were getting in the linked issue, but when using --jsxImportSource with TS, the tsconfig.json will need to have different values. While users in all likelihood will have a tsconfig.json that's correctly configured in that case, if they do not, their typings will contain any where JSX references should be.

Setting these configurations ourselves, at least as defaults, allows us to make fewer assumptions about users' configs that are external to Microbundle (and hopefully will raise fewer issues).

jsxImportSource: options.jsxImportSource,
}
: {
jsx: 'preserve',
Copy link
Owner

Choose a reason for hiding this comment

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

should this be preserve, or react?

Copy link
Collaborator Author

@rschristian rschristian Aug 11, 2022

Choose a reason for hiding this comment

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

preserve should be correct, though from my quick tests I don't think it makes a difference either way.

preserve is to be used when having another tool transpile JSX (e.g., babel), which is what we do. react on the other hand has TSC transpile it itself. TS Docs

I think, at best, react would do nothing, and at worst, it'd slow down type generation due to trying to handle JSX which we don't need. No one's raised any issues regarding preserve so I'd assume it's fine.

Edit: Depends on what we want, I suppose. I'd prefer to have TS handle as little as possible and preserve is already the default behavior. I didn't originally realize we did have TS handle JSX.

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.

react-jsx does not work with Preact
2 participants