preactjs / preset-vite

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

bug?: When the preact-render-to-string dependency does not exist, the error that occurs is vague #101

Closed rxliuli closed 9 months ago

rxliuli commented 9 months ago

I used depcheck to identify and remove unused dependencies, among which preact-render-to-string was also deleted. I didn't realize that it is essential for prerender. However, I encountered a very strange error. Perhaps it's a good idea to add a check to see if this dependency exists when prerender.enable === true and throw a more explicit exception.

[preact:prerender] d is not a function
✓ built in 130ms
error during build:
TypeError: d is not a function

Reproducible Example

git clone https://github.com/rxliuli/preact-ssg-demo.git
cd preact-ssg-demo
pnpm i
pnpm build
rschristian commented 9 months ago

That doesn't come from us, but your usage.

preact-render-to-string is an optional dependency of preact-iso, who's prerender function you're using in your demo. The issue, however, is that we just call your prerender function -- we don't use preact-render-to-string in the plugin itself at all. Whatever you return is the HTML we use (which makes this extra neat as you can prerender other frameworks too).

Take the following (completely valid) example:

export async function prerender(data) {
    return { html: '<h1>Hello World!</h1>' };
}

While most Preact apps will prerender via preact-render-to-string, it's not necessarily guaranteed and we have no legitimate requirement that it be installed because of this. As the dep is on your side (through the prerender() function you define), this is more of a user-side issue.

I agree that message isn't clear, but I'm not sure what can be done.

rxliuli commented 9 months ago

I brought up this issue in the vitejs discussion forum to see what the vitejs community has to say. https://github.com/vitejs/vite/discussions/15725

rschristian commented 9 months ago

Going to close this out as it looks like it's being properly addressed in Vite itself -- certainly feel free to comment if there's something we need to do here. Happy to reopen and investigate further.