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

fix(solid, solid-vite): preset.js: module is not defined #19

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JellyBrick
Copy link

@bichikim
Copy link

bichikim commented Jan 21, 2025

If executed on Node.js version 22.x.x, the same error occurs.

In the 20.x.x version, it runs without any errors.

@ShadowRZ
Copy link

@bichikim Curiously, this problem only appears when Node.js has its experimental support for require() ESM modules enabled, as when I run Storybook with NODE_OPTIONS="--no-experimental-require-module" the problem doesn't appear

@JellyBrick
Copy link
Author

JellyBrick commented Jan 25, 2025

@bichikim Curiously, this problem only appears when Node.js has its experimental support for require() ESM modules enabled, as when I run Storybook with NODE_OPTIONS="--no-experimental-require-module" the problem doesn't appear

After v22.10.0 v22.12.0, require(esm) enabled by default.
see https://socket.dev/blog/node-js-delivers-first-lts-with-require-esm-enabled

@ShadowRZ
Copy link

After v22.10.0, require(esm) enabled by default.

Indeed, I'm also aware of this after I commented, BTW it's v22.12.0, not v22.10.0.

Hope the PR got merged and a new beta is released.

@JellyBrick
Copy link
Author

@JonahPlusPlus Could you check this PR?

@JonahPlusPlus
Copy link
Collaborator

It seems fine (still gonna test it locally to verify the build output), but when comparing it to the Storybook repo, I noticed they are still using the require syntax. Does Storybook for React still work? Could I get a QRD of any PRs on that side?

(I want to make sure this project is consistent with practices on the main repo)

@JonahPlusPlus
Copy link
Collaborator

This also should change the .nvmrc version to 22.13.1.

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.

4 participants