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

Importing solid-js/web (as ESM, from Node.js) throws a SyntaxError #1984

Closed
vinsonchuong opened this issue Dec 9, 2023 · 11 comments
Closed

Comments

@vinsonchuong
Copy link

vinsonchuong commented Dec 9, 2023

Describe the bug

When I import solid-js/html as an ES module from Node.js, I get the following error:

SyntaxError: The requested module 'solid-js/web' does not provide an export named 'Aliases'

Your Example Website or App

n/a

Steps to Reproduce the Bug or Issue

For example, in the Node.js REPL:

>> node
Welcome to Node.js v21.1.0.
Type ".help" for more information.
> await import('solid-js/html')
file:///home/vinsonchuong/projects/test-tube/node_modules/solid-js/html/dist/html.js:15
  Aliases,
  ^^^^^^^
Uncaught:
SyntaxError: The requested module 'solid-js/web' does not provide an export named 'Aliases'

Expected behavior

I expect the import to succeed and not throw an error.

Screenshots or Videos

No response

Platform

  • OS: Linux
  • Browser: Running in Node.js v21.1.0
  • Version: 1.8.7

Additional context

I'm looking to write some custom tooling for writing automated tests from Node.js and was surprised to see this error.

After some searching, I found this issue: #1483, which clued me into the fact that there are separate builds for the browser and for Node.js.

It looks like because the html tag function returns a DOM Node, solid-js/html is a browser-only package. I am, however, able to proceed if I configure Node.js to prefer browser packages:

>> node --conditions browser
Welcome to Node.js v21.4.0.
Type ".help" for more information.
> await import('solid-js/html')
[Module: null prototype] { default: [Function: html] }

Because solid-js/html imports solid-js/web, there's no way to specify that the browser packages should be used without passing in arguments to Node.js. To make this easier, could solid-js/html directly import the browser build of solid-js/web instead? There are probably trade-offs to doing that, but I'm hoping to have easy ways to run Solid from a Node.js + JSDOM environment.

@ryansolid
Copy link
Member

So far we've pushed people to use "browser" export conditions for this. Like in solid-jest when doing testing we use the browser condition. We could make it use the browser version. Although there are 2 versions of Solid as well, and it is less clear what version you want. Like testing I want the browser reactive version, but for doing some sort of SSR you probably want the inert server version. To be fair right now because they all connect to the same condition it isn't that configurable.

Not sure. What I'm getting at it is this probably won't work correctly for everyone unless we treat it as browser across the board which can only be done through conditions as web internally doesn't wire to browser version of solid etc... It is interesting problem.

@vinsonchuong
Copy link
Author

vinsonchuong commented Dec 17, 2023

Makes sense. It's pretty tough to hit every usecase gracefully.

When reading this section in solid-jest, I noticed that a user can choose the right preset to get what I assume to be the build of Solid that they want.

One of the reasons I'm using solid-js/html is, for simple projects, I tend towards not having a compiler.

So, I'm wondering if a similar concept can be applied here. Perhaps by importing something like solid-js/html/browser or solid-js/web/browser, we can get the hardcoded browser builds. I see that solid-js/web ends up importing solid-js. So, if solid-js/web/browser ended up importing solid-js/browser, that might be a graceful way to hit this usecase.

What do you think?

Edit: For additional context, I've found a decent workaround via a custom Node.js loader hook that rewrites imports for solid-js/* to the version that I want. I'm actually not able to use --conditions browser because that affects all packages, and I want the Node.js version of some packages.

A loader hook is a reasonable path, assuming that the test runner in use allows configuring Node.js arguments.

@dan-lee
Copy link

dan-lee commented Dec 19, 2023

I am hitting this issue in Vite/Vitest and am still looking for a workaround. Setting resolve.conditions is too intrusive in my use case and causes other side effects.
So a direct kind of import would also be very helpful to me!

@ryansolid
Copy link
Member

I am hitting this issue in Vite/Vitest and am still looking for a workaround. Setting resolve.conditions is too intrusive in my use case and causes other side effects. So a direct kind of import would also be very helpful to me!

I think you are going have to depend aliasing then because this is a multi-package concern. While any package could just import from the right source (we expose dist) our core libraries which reference each other would not. Export conditions make sense here or bundler aliasing makes sense here but hard-coding the libraries doesn't particularly.

It doesn't really extend out well into the ecosystem either. We tend to ship as source to give maximum compilation flexibility to the end user so like intermediate libraries aren't going to do the combinatorics as well.

I think we should look at export conditions here a bit more and see if we can be more helpful in some way. Like we have a "server" export condition when people need that build regardless of node etc.. Maybe there is a way to get client that doesn't have other implications. That being said this hasn't been a problem for a lot of projects because the solid parts could be built with that setting and then the setting could be absent from the runtime when running.

@trusktr
Copy link
Contributor

trusktr commented Sep 1, 2024

I can't import Solid's html in a Solid Start app if SSR is enabled:

import html from `solid-js/html`

Causes:

7:15:10 PM [vite] Error when evaluating SSR module /src/elements/Scroller.ts: failed to import "solid-js/html"
|- file:///Users/trusktr/src/showcase/node_modules/solid-js/html/dist/html.js:1
import { effect, style, insert, untrack, spread, createComponent, delegateEvents, classList, mergeProps, dynamicProperty, setAttribute, setAttributeNS, addEventListener, Aliases, getPropAlias, Properties, ChildProperties, DelegatedEvents, SVGElements, SVGNamespace } from 'solid-js/web';
                                                                                                                                                                          ^^^^^^^
SyntaxError: The requested module 'solid-js/web' does not provide an export named 'Aliases'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:131:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:213:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:308:24)
    at async nodeImport (file:///Users/trusktr/src/showcase/node_modules/vite/dist/node/chunks/dep-BzOvws4Y.js:52954:15)
    at async ssrImport (file:///Users/trusktr/src/showcase/node_modules/vite/dist/node/chunks/dep-BzOvws4Y.js:52812:16)
    at async eval (/Users/trusktr/src/showcase/src/elements/Scroller.ts:4:31)
    at async instantiateModule (file:///Users/trusktr/src/showcase/node_modules/vite/dist/node/chunks/dep-BzOvws4Y.js:52870:5)

Solid's html code causes the problem here:

https://github.com/solidjs/solid/blob/main/packages/solid/html/src/index.ts#L17

The solution is Solid should export Aliases regardless.

And if Aliases should not be used on the server side, then a runtime error should be thrown instead of having mismatching exports (people would be able to easily guard against the error with if (globalThis.window) useAliases(Aliases)).

@trusktr
Copy link
Contributor

trusktr commented Sep 1, 2024

Code in dom-expressions can be made to workaround Aliases missing at runtime, if needed. For example this code here,

https://github.com/ryansolid/dom-expressions/blob/b0016751ad0149e61c93337ce10c135f3f8efe90/packages/lit-dom-expressions/src/index.ts#L203

already does this

r.Aliases[name] || name

and let's say that Aliases were to at least be exported as undefined (I'm not suggesting that's the best solution, but for example)) then that code can be updated to

r.Aliases?.[name] || name

and this creepy crawly can be squashed. 🐛🥾

Similarly any other code that might be relying on Aliases can be updated.

Screenshot 2024-08-31 at 7 48 04 PM

@trusktr
Copy link
Contributor

trusktr commented Sep 1, 2024

@trusktr
Copy link
Contributor

trusktr commented Sep 1, 2024

Not only does ryansolid/dom-expressions#345 fix this error for Solid Start, but it fixes the error for all SSR frameworks.

For example, I've had this error turning up for me and other people where were trying to use Lume in Svelte Kit, Next, etc. Gonna be really nice to have this one finally fixed!

@trusktr
Copy link
Contributor

trusktr commented Sep 1, 2024

Aha! Well I've verified and tested that this is fixed in the above PR ^, as far as import solid-js/html goes.

But now that that's out of the way, I've discovered more similar issues in separate packages that I'll also fix:

SyntaxError: The requested module 'solid-js/store' does not provide an export named 'modifyMutable'

Error with SSR enabled, no error with SSR disabled.

I'll update this PR to also include updates for other sub-folders such as solid-js/store/.

@trusktr
Copy link
Contributor

trusktr commented Sep 2, 2024

I've updated #2269 and can confirm that the updated build of solid-js in a Solid Start app prevents any SSR errors due to imports (the app code then runs logic on the client conditionally if needed), and I confirmed that SSR is working (HTML payload arrives from the server). 🎉

I am still having a hydration error of some kind, but imports are working great now.

@ryansolid
Copy link
Member

Yeah merged into next. Will be in 1.9.

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

No branches or pull requests

4 participants