preactjs / preset-vite

Preset for using Preact with the vite bundler
https://npm.im/@preact/preset-vite
MIT License
262 stars 26 forks source link

`Uncaught ReferenceError: React is not defined` and emits `@babel/plugin-transform-react-jsx-source` warning. #100

Closed aspizu closed 9 months ago

aspizu commented 10 months ago

I have created a module for components, at https://github.com/aspizu/bloom.

I try to use it in a preact project, but it fails with the React is not defined error. Also, it emits this warning,

Add @babel/plugin-transform-react-jsx-source to get a more
detailed component stack. Note that you should not add it
to production builds of your App for bundle size reasons.

Minimal reproduction:

{
  "name": "bloom-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "vite": "vite"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "@preact/preset-vite": "^2.8.1",
    "stylus": "^0.62.0",
    "vite": "^5.0.12"
  },
  "dependencies": {
    "bloom": "github:aspizu/bloom",
    "preact": "^10.19.3"
  }
}
import preact from "@preact/preset-vite";
import { defineConfig } from "vite";

export default defineConfig({
  plugins: [preact()],
});
import { Button } from "bloom/components";

export default function App() {
  return (
    <>
      <Button>Hello</Button> // This will cause the errors.
    </>
  );
}
marvinhagemeister commented 10 months ago

@aspizu In most scenarios where this message is logged to the terminal there is an error printed right after it. Can you share that too, or even better the full terminal output?

aspizu commented 10 months ago
Uncaught ReferenceError: React is not defined
    Button http://localhost:5173/node_modules/.vite/deps/bloom_components.js?v=1b726c42:802
    O http://localhost:5173/node_modules/.vite/deps/chunk-JXXRVW4W.js?v=1b726c42:274
    L http://localhost:5173/node_modules/.vite/deps/chunk-JXXRVW4W.js?v=1b726c42:193
    C http://localhost:5173/node_modules/.vite/deps/chunk-JXXRVW4W.js?v=1b726c42:77
    L http://localhost:5173/node_modules/.vite/deps/chunk-JXXRVW4W.js?v=1b726c42:195
    C http://localhost:5173/node_modules/.vite/deps/chunk-JXXRVW4W.js?v=1b726c42:77
    L http://localhost:5173/node_modules/.vite/deps/chunk-JXXRVW4W.js?v=1b726c42:195
    q http://localhost:5173/node_modules/.vite/deps/chunk-JXXRVW4W.js?v=1b726c42:278
    <anonymous> http://localhost:5173/src/index.tsx:7

Contents of bloom_components.js

// node_modules/.pnpm/github.com+aspizu+bloom@7b4932227c79fabd3713c32c4b37a0bb06cc9e83/node_modules/bloom/src/components/Button.tsx
function Button({
  class: className,
  children,
  variant,
  disabled,
  iconLeft,
  iconRight,
  ...props
}) {
  const $3 = component(className, "Button", variant, {
    disabled,
    iconLeft,
    iconRight
  });
  return React.createElement("button", { ...props, class: $3() }, children);
}

vs App.tsx


import { createHotContext as __vite__createHotContext } from "/@vite/client";import.meta.hot = __vite__createHotContext("/src/App.tsx");        import "/node_modules/.pnpm/@prefresh+core@1.5.2_preact@10.19.3/node_modules/@prefresh/core/src/index.js?v=1b726c42";        import { flush as flushUpdates } from "/node_modules/.pnpm/@prefresh+utils@1.2.0/node_modules/@prefresh/utils/src/index.js?v=1b726c42";        let prevRefreshReg;        let prevRefreshSig;        if (import.meta.hot) {          prevRefreshReg = self.$RefreshReg$ || (() => {});          prevRefreshSig = self.$RefreshSig$ || (() => (type) => type);          self.$RefreshReg$ = (type, id) => {            self.__PREFRESH__.register(type, "/home/aspizu/Documents/Projects/bloom-test/src/App.tsx" + " " + id);          };          self.$RefreshSig$ = () => {            let status = 'begin';            let savedType;            return (type, key, forceReset, getCustomHooks) => {              if (!savedType) savedType = type;              status = self.__PREFRESH__.sign(type || savedType, key, forceReset, getCustomHooks, status);              return type;            };          };        }        var _jsxFileName = "/home/aspizu/Documents/Projects/bloom-test/src/App.tsx";
import { Button } from "/node_modules/.vite/deps/bloom_components.js?v=1b726c42";
import { jsxDEV as _jsxDEV } from "/node_modules/.vite/deps/preact_jsx-dev-runtime.js?v=1b726c42";
import { Fragment as _Fragment } from "/node_modules/.vite/deps/preact_jsx-dev-runtime.js?v=1b726c42";
export default function App() {
  return _jsxDEV(_Fragment, {
    children: _jsxDEV(Button, {
      children: "Hello"
    }, void 0, false, {
      fileName: _jsxFileName,
      lineNumber: 6,
      columnNumber: 7
    }, this)
  }, void 0, false);
}```
aspizu commented 10 months ago

If I link the package locally, then it works.

"bloom": "link:../bloom"

But, it doesn't work if I install from github.

aspizu commented 10 months ago

I added this to ./node_modules/@preact/preset-vite/dist/esm/index.mjs

async transform(code, url) {
    // Ignore query parameters, as in Vue SFC virtual modules.
    const { id } = parseId(url);
    console.log(id)
    if (!shouldTransform(id))
        return;
    console.log("SHOULD TRANSFORM ^^^")

output after running vite build

/home/aspizu/Documents/Projects/bloom-test/src/App.tsx
SHOULD TRANSFORM ^^^
/home/aspizu/Documents/Projects/bloom-test/node_modules/.pnpm/github.com+aspizu+bloom@7b5814022baa1c191a63a6afb8233f79692dd2b8/node_modules/bloom/src/components/index.ts
/home/aspizu/Documents/Projects/bloom-test/node_modules/.pnpm/github.com+aspizu+bloom@7b5814022baa1c191a63a6afb8233f79692dd2b8/node_modules/bloom/src/components/Badge.tsx

so the code from bloom package isn't getting transformed.

aspizu commented 10 months ago

I'm trying to play with the include and exclude options, but they seem to do nothing.

rschristian commented 10 months ago

so the code from bloom package isn't getting transformed.

Which is correct, it shouldn't be. Shipping .ts or .tsx like that to consumers isn't going to be a valid choice in most tools. Transpile ahead of time.

As for aliasing, that's not done as part of transform(), but as part of the resolution. See here. Pulling from GitHub might causes problems with that, would probably avoid it myself. That sounds like it'd be a Vite issue though.

aspizu commented 9 months ago

this is also a problem if im developing a components library which uses hooks, I don't want to compile my library.

rschristian commented 9 months ago

I don't want to compile my library.

I don't think this is something we should support. Status quo is to transpile your library before distribution, as shown on NPM, etc. The reason for this is pretty simple: you can transpile your library once, up front, or thousands of times every time you run/consume your library.

Choosing to forgo that is fine, but it places the burden on you to transpile it yourself, either by patching the preset here or adding your own plugin to the compilation that'll handle your assets from node_modules

aspizu commented 9 months ago

how would I make it work with both react and preact, and also work if the user doesn't use preact signals?

rschristian commented 9 months ago

Aliasing covers the first bit, but you'll need to clarify what you're after for the second. Your library can use signals without requiring your end users use them as well as the other way around. They're just an optional state primitive.

If your concern is over allowing a user to pass a signal or a primitive to your library, you can use something like this:

const signalMaybeUnwrapper = (maybeSignal) =>
    maybeSignal instanceof Signal
      ? maybeSignal.peek()
      : maybeSignal;

if (signalMaybeUnwrapper(count) !== 'example') {
    ...
}
aspizu commented 9 months ago

why peek over .value ?

rschristian commented 9 months ago

Just an example, .peek() and .value can both be valid, depending on what you need.

aspizu commented 9 months ago

doesn't .value require compilation?

rschristian commented 9 months ago

No? .value just unwraps a signal to the value inside. Signals are not a compilation-enabled primitive.

aspizu commented 9 months ago

alright thank you, but i still feel like you should support compilation for node modules

rschristian commented 9 months ago

I think we can close this out then.

There might be a legitimate issue with aliasing in modules pulled directly from GitHub but that is likely an upstream issue in Vite -- nothing we do, from what I can see, should have any impact.