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(create-waku): downgrade ncc #938

Merged
merged 1 commit into from
Oct 4, 2024
Merged

fix(create-waku): downgrade ncc #938

merged 1 commit into from
Oct 4, 2024

Conversation

dai-shi
Copy link
Owner

@dai-shi dai-shi commented Oct 4, 2024

@vercel/ncc@0.38.2 seems to break create-waku's notifyUpdate.

Here's the error log:

TypeError: undefined is not a function
    at notifyUpdate (file:///.../node_modules/create-waku/dist/index.js:30:110825)

@himself65 @ojj1123 do you think it's fixable?

In the meantime, let's downgrade ncc.

Copy link

vercel bot commented Oct 4, 2024

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

1 Skipped Deployment
Name Status Preview Updated (UTC)
waku ⬜️ Ignored (Inspect) Oct 4, 2024 3:58am

@dai-shi dai-shi marked this pull request as ready for review October 4, 2024 03:58
Copy link

codesandbox-ci bot commented Oct 4, 2024

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.

@dai-shi dai-shi merged commit 2613e58 into main Oct 4, 2024
28 checks passed
@dai-shi dai-shi deleted the downgrade-ncc branch October 4, 2024 04:25
@ojj1123
Copy link
Contributor

ojj1123 commented Oct 4, 2024

Is it fixed? or need any investigating? If so, how could I reproduce it?

@dai-shi
Copy link
Owner Author

dai-shi commented Oct 4, 2024

It's not fixed. If you update ncc to the latest version, you can reproduce it. If you check dist/index.js, createRequire is gone.

@himself65
Copy link
Contributor

we can replace ncc to other bundler like tsup, bunchee or something, they are not using webpack, webpack has too many hakcs on imports

@dai-shi
Copy link
Owner Author

dai-shi commented Oct 4, 2024

I hope we can replace it with more lightweight one. What was the reason we need bundling in the first place again?

@ojj1123
Copy link
Contributor

ojj1123 commented Oct 4, 2024

When merging #933, @vercel/ncc was upgraded in 0.38.2.

we can replace ncc to other bundler like tsup, bunchee or something, they are not using webpack, webpack has too many hakcs on imports

You mean that @vercel/ncc depends on webpack. Btw why did you use ncc? related PR #330

This was referenced Oct 18, 2024
@dai-shi
Copy link
Owner Author

dai-shi commented Oct 18, 2024

@ojj1123 @himself65 just opened a new issue #973.

dai-shi added a commit that referenced this pull request Oct 18, 2024
oops, I forgot to keep the ncc version. the previous one: #938
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.

3 participants