Allow JSX Import Source to be overwritten #47

Interpause · 2022-04-14T14:28:04Z

This pull request solves #46 by adding jsxImportSource to PreactPluginOptions. As a bonus, I changed prepublishOnly to prepare, which allows the package to be installed directly via Git and should not affect its behaviour when being published to npm.

package.json

Interpause · 2022-07-03T01:58:37Z

bump

…actjs#48) * add named export to improve `"module": "nodenext"` compatibility * add proper types export * Update src/index.ts Co-authored-by: Jovi De Croock <decroockjovi@gmail.com> Co-authored-by: Jovi De Croock <decroockjovi@gmail.com>

marvinhagemeister

This PR has the right spirit, but does a whole lot more than just allowing jsxImportSource to be overwritten.

Interpause · 2022-09-11T00:54:26Z

This PR has the right spirit, but does a whole lot more than just allowing jsxImportSource to be overwritten.

Is this referring to the additional 3 commits caused by fast forwarding the pull request to main?

rschristian · 2022-09-11T08:16:49Z

You need to rebase your changes. The diff is quite mangled.

Interpause · 2022-09-17T04:00:02Z

Should be fixed now.

JoviDeCroock · 2022-09-17T08:21:20Z

@Interpause the comment to leave prePublish in still stands 😅 we want this so when we use npm publish we are sure we can't forget to use the latest build

rschristian · 2022-09-17T08:31:47Z

@JoviDeCroock FWIW, prepare does run before a publish: https://docs.npmjs.com/cli/v8/using-npm/scripts#life-cycle-scripts

I would argue against prepare still though as there's no reason we need to run a build immediately after installing deps (which it does). That's just a nuisance, IMO.

Edit: I guess it facilitates installing from git, but meh. Do people do that often enough?

marvinhagemeister · 2022-09-18T07:03:08Z

package.json

@@ -17,7 +17,8 @@
 		"dev": "vite demo",
 		"dev:build": "vite build demo",
 		"build": "rimraf dist && tsc && tsc -p tsconfig.cjs.json && node tools/postbuild.mjs",
-		"prepublishOnly": "npm run build"
+		"prepublishOnly": "npm run build",
+		"prepare": "npm run build"


This causes the build command to be run whenever a user installs our vite preset. Is this change necessary to allow JSX Import Source to be overwritten?

According to the docs, I think it's only when the preset is installed through git? Which, of course, would require build be ran.

I think it's not just that:

Runs on local npm install without any arguments

In either way this change doesn't seem necessary to allow JSX import source to be overwritten.

I think it's not just that:

microbundle has a prepare script, for example, and it doesn't run on install (nor could it). "local" means this project, as in, if you were to clone this repo and run npm install.

Not the best wording from NPM, to be fair.

In either way this change doesn't seem necessary to allow JSX import source to be overwritten.

Yeah totally agree there, and I generally think it's a super annoying life cycle script anyways.

I stand corrected! Looks like I made the wrong assumption.

I am guessing to keep the pull request pure, you want to remove the changes to package.json?

That, but also as it stands, you haven't provided an explanation for the change. Why are we adding a new script?

While we can guess about your intentions (as we've done above), for right now, it's a change without any rhyme nor reason.

To be fair, the prepare script isn't needed if the package is installed from npm. However, as I needed to overwrite the import source for a project, and did not want to publish to npm, I added the prepare script so that installing via git would be possible.

Interpause · 2022-10-09T12:48:35Z

Say, its been a while. Actually, I think the changes made to allow configuring jsxImportSource is quite minor, such that it probably would not be too much trouble for a maintainer to just include it in a commit. If so, would it be fine to close this PR?

o-alexandrov · 2023-03-05T16:13:16Z

Could you please consider to merge the PR, as the changes had been cleaned up

o-alexandrov · 2023-03-05T16:16:36Z

src/index.ts

@@ -130,7 +135,7 @@ function preactPlugin({
 							: "@babel/plugin-transform-react-jsx-development",
 						{
 							runtime: "automatic",
-							importSource: "preact",
+							importSource: jsxImportSource ?? "preact",


nullish coalescing while having no benefit over || operator in this case, still shouldn't break anyone's environment, as it's supported in Node.js 14 (oldest version that hasn't reached EOL)

Interpause added 2 commits April 14, 2022 22:17

allow jsx import source to be configured

2d6d079

allow package to be installed via git

90069cf

JoviDeCroock approved these changes Apr 15, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

kdubiel approved these changes Jun 20, 2022

View reviewed changes

Interpause mentioned this pull request Aug 7, 2022

Yarn Monorepo + PnP mode + Modified React ImportSource + aliasing to Preact = Build Failure vitejs/vite#9570

Closed

7 tasks

sapphi-red and others added 3 commits August 7, 2022 16:05

fix: support vite v3 (preactjs#50)

202fe1c

2.3.0 (preactjs#53)

767e93f

marvinhagemeister requested changes Sep 4, 2022

View reviewed changes

Merge branch 'main' into overwrite-import-source

c09efa4

Update package.json

a26a128

marvinhagemeister reviewed Sep 18, 2022

View reviewed changes

remove "prepare" script

f329db6

o-alexandrov reviewed Mar 5, 2023

View reviewed changes

JoviDeCroock approved these changes Oct 13, 2023

View reviewed changes

JoviDeCroock requested a review from marvinhagemeister October 13, 2023 07:16

marvinhagemeister approved these changes Oct 13, 2023

View reviewed changes

JoviDeCroock merged commit 8223769 into preactjs:main Oct 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow JSX Import Source to be overwritten #47

Allow JSX Import Source to be overwritten #47

Interpause commented Apr 14, 2022

Interpause commented Jul 3, 2022

marvinhagemeister left a comment

Interpause commented Sep 11, 2022

rschristian commented Sep 11, 2022

Interpause commented Sep 17, 2022

JoviDeCroock commented Sep 17, 2022 •

edited

Loading

rschristian commented Sep 17, 2022 •

edited

Loading

marvinhagemeister Sep 18, 2022

rschristian Sep 18, 2022

marvinhagemeister Sep 18, 2022

rschristian Sep 18, 2022 •

edited

Loading

marvinhagemeister Sep 18, 2022 •

edited

Loading

Interpause Sep 24, 2022

rschristian Sep 24, 2022

Interpause Sep 25, 2022

Interpause commented Oct 9, 2022

o-alexandrov commented Mar 5, 2023

o-alexandrov Mar 5, 2023 •

edited

Loading

Allow JSX Import Source to be overwritten #47

Allow JSX Import Source to be overwritten #47

Conversation

Interpause commented Apr 14, 2022

Interpause commented Jul 3, 2022

marvinhagemeister left a comment

Choose a reason for hiding this comment

Interpause commented Sep 11, 2022

rschristian commented Sep 11, 2022

Interpause commented Sep 17, 2022

JoviDeCroock commented Sep 17, 2022 • edited Loading

rschristian commented Sep 17, 2022 • edited Loading

marvinhagemeister Sep 18, 2022

Choose a reason for hiding this comment

rschristian Sep 18, 2022

Choose a reason for hiding this comment

marvinhagemeister Sep 18, 2022

Choose a reason for hiding this comment

rschristian Sep 18, 2022 • edited Loading

Choose a reason for hiding this comment

marvinhagemeister Sep 18, 2022 • edited Loading

Choose a reason for hiding this comment

Interpause Sep 24, 2022

Choose a reason for hiding this comment

rschristian Sep 24, 2022

Choose a reason for hiding this comment

Interpause Sep 25, 2022

Choose a reason for hiding this comment

Interpause commented Oct 9, 2022

o-alexandrov commented Mar 5, 2023

o-alexandrov Mar 5, 2023 • edited Loading

Choose a reason for hiding this comment

JoviDeCroock commented Sep 17, 2022 •

edited

Loading

rschristian commented Sep 17, 2022 •

edited

Loading

rschristian Sep 18, 2022 •

edited

Loading

marvinhagemeister Sep 18, 2022 •

edited

Loading

o-alexandrov Mar 5, 2023 •

edited

Loading