Skip to content

Conversation

ha1fstack
Copy link
Contributor

@ha1fstack ha1fstack commented Feb 25, 2025

relates to #321

node:path's join on windows uses \\ separator but node-glob by default accepts only /

Copy link

changeset-bot bot commented Feb 25, 2025

🦋 Changeset detected

Latest commit: 688c369

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@opennextjs/cloudflare Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Feb 25, 2025

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/cloudflare@413

commit: 688c369

@vicb
Copy link
Contributor

vicb commented Feb 25, 2025

Thanks for the PR 🙏

Could you please

  • add a patch changeset (run pnpm changeset and follow the prompt)
  • run pnpm fix to format the code

@vicb
Copy link
Contributor

vicb commented Feb 26, 2025

@ha1fstack have you tested this successfuly on windows - I can not really tell from https://www.npmjs.com/package/glob#windows if that fixes the issue?

@ha1fstack
Copy link
Contributor Author

@ha1fstack
Copy link
Contributor Author

ha1fstack commented Feb 26, 2025

for context, the linked issue thread has two separate problems

  • [WARNING] Comparison with -0 using the "===" operator will also match 0 [equals-negative-zero]

as you commented this should be fixed on upstream and it does not crash the build.

  • X [ERROR] service core:user:workers-next-gpt-trial: Uncaught Error: Unknown loadManifest: /.next/server/next-font-manifest.json

the pr fixes this issue - on windows system node:path uses \\ as separator but glob by default only accepts /, so glob returns empty array.

windowsPathsNoEscape option replaces double backslash with single forward slash.

@ha1fstack
Copy link
Contributor Author

ha1fstack commented Feb 26, 2025

I had some misunderstandings, this PR is unrelated to the linked issue's parent post ([WARNING] Comparison with -0 using...).
It fixes separate problem that crashes the build on windows systems, mentioned in issue thread.

cloudflare/workers-sdk#8159
this should be the linked issue (but it's on workers-sdk repo), since it was not fixed by #394.

@vicb vicb merged commit 01e2bfb into opennextjs:main Feb 26, 2025
7 checks passed
@vicb
Copy link
Contributor

vicb commented Feb 26, 2025

Thanks @ha1fstack

@ha1fstack
Copy link
Contributor Author

Thank you for prompt response and fixup!

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.

2 participants