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(build): dyn import in skypack (native ESM) #456

Merged
merged 3 commits into from
May 20, 2022
Merged

Conversation

ahaoboy
Copy link
Contributor

@ahaoboy ahaoboy commented May 10, 2022

fix: dyn import in skypack (close: #452)
I compared the versions that works, this is the difference

error

import {useSyncExternalStore} from "/-/[email protected]/dist=es2019,mode=imports/unoptimized/shim/index.js";

ok

import de from "/-/[email protected]/dist=es2019,mode=imports/unoptimized/shim/index.js";

fix: dyn import in skypack (close: pmndrs#452)
@vercel
Copy link

vercel bot commented May 10, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
valtio ✅ Ready (Inspect) Visit Preview May 20, 2022 at 11:04AM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 10, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 490ac59:

Sandbox Source
React Configuration
React Typescript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration

@ahaoboy
Copy link
Contributor Author

ahaoboy commented May 10, 2022

I'm not sure if it will affect #410

@dai-shi
Copy link
Member

dai-shi commented May 11, 2022

Thanks for the investigation.
This looks like a valid workaround until use-sync-external-store provides ESM.
I'll add a commit for some comments.

@dai-shi dai-shi changed the title fix: dyn import in skypack (close: #452) fix(build): dyn import in skypack (close: #452) May 11, 2022
@dai-shi dai-shi changed the title fix(build): dyn import in skypack (close: #452) fix(build): dyn import in skypack (native ESM) May 11, 2022
@dai-shi
Copy link
Member

dai-shi commented May 11, 2022

@dai-shi
Copy link
Member

dai-shi commented May 19, 2022

I'm a little confused. Why doesn't 'react' cause the same issue?

import { useCallback, useDebugValue, useEffect, useMemo, useRef } from 'react'

@dai-shi
Copy link
Member

dai-shi commented May 20, 2022

Maybe because it's a peer dependency.

@dai-shi
Copy link
Member

dai-shi commented May 20, 2022

I'm not 100% confident, but let's merge this and ship it. We will see then.

@dai-shi dai-shi merged commit c4b0151 into pmndrs:main May 20, 2022
@dai-shi
Copy link
Member

dai-shi commented May 30, 2022

@ahaoboy I'm thinking about reverting this because @s-cork reported that it didn't fix. facebook/react#24590

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jun 6, 2022

I try 1.6.1, it works...
image

@s-cork
Copy link

s-cork commented Jun 6, 2022

When I do that I get:

use-sync-external-store-shim.development.js:17 Uncaught ReferenceError: process is not defined
    at use-sync-external-store-shim.development.js:16:3
    at createCommonjsModule (use-sync-external-store-shim.development.js:4:10)
    at use-sync-external-store-shim.development.js:15:22

When I look at what's trying to be imported from use-sync-external-store-shim there are some references to (process.env.NODE_ENV !== "production")

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jun 6, 2022

I've made an example, if it's what you need? or if you can give some specific details of the problem
@s-cork
Web Platform (forked) - StackBlitz

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jun 6, 2022

When I do that I get:

use-sync-external-store-shim.development.js:17 Uncaught ReferenceError: process is not defined
    at use-sync-external-store-shim.development.js:16:3
    at createCommonjsModule (use-sync-external-store-shim.development.js:4:10)
    at use-sync-external-store-shim.development.js:15:22

When I look at what's trying to be imported from use-sync-external-store-shim there are some references to (process.env.NODE_ENV !== "production")

You should add some code before your script tag, like this:

    <script>
      globalThis.process = { env: { NODE_ENV: "dev" } };
    </script>

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.

using valtio with skypack.dev
3 participants