Skip to content

Conversation

@benoitv-code
Copy link
Contributor

@benoitv-code benoitv-code commented Jun 14, 2025

Fix server-side testing with vitest/ssr

Initial error

Initial error when running vitest in ssr mode, since [email protected] (introduced by #173, reported by #211):

Error: renderToString is not supported in the browser, returning undefined
    at throwInBrowser (.../node_modules/solid-js/web/dist/dev.js:1089:15)

Proposed fix

  • This PR proposes not adding 'browser' if options.ssr is set

I am not familiar with the code, and I am not sure if it could lead to issues for other setups.
An alternative could be to only add 'browser' if the conditions don't already include 'node':

-...(isTestMode && !opts.isSsrTargetWebworker ? ['browser'] : []),
+...(isTestMode && !opts.isSsrTargetWebworker && !config.resolve.conditions.includes('node') ? ['browser'] : []),

Let me know which solution seems to be the best.

Additional note

The opts.isSsrTargetWebworker check seems to be inconsistant in the original code, I've left it untouched as I am not sure what is the intent there:

// Original code: Uses `client` conditions if isSsrTargetWebworker is set
if (config.consumer === 'client' || name === 'client' || opts.isSsrTargetWebworker) {

// Original  code: Later, not using `browser` condition if isSsrTargetWebworker set
...(isTestMode && !opts.isSsrTargetWebworker ? ['browser'] : []),

Can original author comment on if the original logic seems consistent? (cc @bluwy @brenelz)

Thanks!


Fixes #211

@benoitv-code
Copy link
Contributor Author

@birkskyum Can you have a look at this PR please? (this prevents me from updating to a version that supports vite 7 :)) Thanks!

@birkskyum birkskyum requested a review from brenelz October 1, 2025 12:41
Copy link
Member

@birkskyum birkskyum left a comment

Choose a reason for hiding this comment

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

I think it looks reasonable, so let's try this. Thanks!

@birkskyum
Copy link
Member

@bluwy , if you have some thoughts on this , I'd love to hear it.

@birkskyum birkskyum merged commit ce00b4b into solidjs:main Oct 1, 2025
@birkskyum
Copy link
Member

It's publishing a patch now - you can check if it works.

@bluwy
Copy link
Contributor

bluwy commented Oct 1, 2025

The change seems fine to me. isSsrTargetWebworker is used for checking SSR builds for webworkers, which tend to be browser-like (in Vite's eyes). So config.consumer === 'client' || name === 'client' || opts.isSsrTargetWebworker is correct in this case.

...(isTestMode && !opts.isSsrTargetWebworker ? ['browser'] : []), is indeed a bit off though (added in #173). Probably should be isTestMode && (!opts.ssr || opts.isSsrTargetWebworker) instead, though usually I think framework plugins shouldn't be meddling with Vitest setup.

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.

Testing ssr (renderToString) in vitest?

3 participants