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 workerd HMR #2019

Merged
merged 2 commits into from
Apr 19, 2024
Merged

Fix workerd HMR #2019

merged 2 commits into from
Apr 19, 2024

Conversation

frandiox
Copy link
Contributor

Related to #1998

The problem doesn't show up anymore with these changes with my current setup but I'm not 100% sure that there are no other ways to reproduce it.

🎩 : Check HMR in any way you can think of... and it should work without errors 🤞

@frandiox frandiox requested a review from a team April 18, 2024 18:44
Copy link
Contributor

@scottdixon scottdixon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good on my end. Was getting an error on every save but now it works great 🙌

@jamalsoueidan
Copy link
Contributor

Possible to test it?

@frandiox frandiox merged commit a335afc into main Apr 19, 2024
15 checks passed
@frandiox frandiox deleted the fd-fix-workerd-hmr branch April 19, 2024 08:42
@frandiox
Copy link
Contributor Author

frandiox commented Apr 19, 2024

@jamalsoueidan Can you try installing @shopify/mini-oxygen@next after this GH action succeeds?
Or you can also do the same change of this PR in your local node_modules/@shopify/mini-oxygen package, it's not a big change.

Edit: released in @shopify/mini-oxygen@0.0.0-next-a335afc-20240419084335

@jamalsoueidan
Copy link
Contributor

Perfect, it works ❤️

@jamalsoueidan
Copy link
Contributor

@frandiox if you run multiply window, it will break as before just to let you know, i can live with one window.

@frandiox
Copy link
Contributor Author

Ohh interesting, I think I know how to fix that but that will need to wait for a week or two ✈️
Feel free to reopen the issue and give more details 👍

@jamalsoueidan
Copy link
Contributor

Ohh interesting, I think I know how to fix that but that will need to wait for a week or two ✈️ Feel free to reopen the issue and give more details 👍

Well, enjoy your holiday 👍🏻 When you come back we will look into that!

@jamalsoueidan
Copy link
Contributor

I notice that the graphql is not auto updated, I have to terminate dev and boot up again.

If graphql not created yet, it will auto-generate the NEW graphql and use it, but if their was any errors, and you fix it, it will not re-generate graphql, so you must terminate and boot up.

@frandiox
Copy link
Contributor Author

frandiox commented May 1, 2024

What do you mean with "the graphql"? A graphql query? Codegen? GraphiQL?

@jamalsoueidan
Copy link
Contributor

jamalsoueidan commented May 1, 2024

I mean that when you define a GraphQL query and fragment, and use them in your code, it will recognize the query and the types, and you will receive all the values. However, if you modify the fragment again—by adding an ID, handle, or something similar—you won't see these attributes return until you reload the development environment

(even if running the codegen in the background)

Same goes with CSS, and doesn't work in multiply connections if testing in cypress, and in the browser, it breaks.

@frandiox
Copy link
Contributor Author

frandiox commented May 1, 2024

if you run multiply window, it will break as before just to let you know, i can live with one window.

I'm testing with 5 tabs open but I can't break it 😞

I mean that when you define a GraphQL query and fragment, and use them in your code, it will recognize the query and the types, and you will receive all the values. However, if you modify the fragment again—by adding an ID, handle, or something similar—you won't see these attributes return until you reload the development environment

I'm trying with something like the following and it updates in all the 5 tabs:

image image

I've also tried with a separate fragment like this:

image

However, changing the fields in the query/fragment or the dom nodes seem to be updating in all the 5 tabs 🤔


cc @scottdixon are you able to reproduce this on your end?

@jamalsoueidan
Copy link
Contributor

The problem happen when importing, not when it's in the same file.

Multiply times I had to terminate the dev to get the latest changes.

@frandiox
Copy link
Contributor Author

frandiox commented May 2, 2024

@jamalsoueidan I think I found the problem with the GraphQL thing, thanks for reporting: #2077

@jamalsoueidan
Copy link
Contributor

I been using hydrogen every single day,,, so testing it is my daily routine hehe

@jamalsoueidan
Copy link
Contributor

A hanging Promise was canceled. This happens when the worker runtime is waiting for a Promise from JavaScript to resolve, but has detected that the Promise cannot possibly ever resolve because all code and events related to the Promise's I/O context have already finished.
[Error: The script will never generate a response.]
A hanging Promise was canceled. This happens when the worker runtime is waiting for a Promise from JavaScript to resolve, but has detected that the Promise cannot possibly ever resolve because all code and events related to the Promise's I/O context have already finished.
Error: Cannot perform I/O on behalf of a different request. I/O objects (such as streams, request/response bodies, and others) created in the context of one request handler cannot be accessed from a different request's handler. This is a limitation of Cloudflare Workers which allows us to improve overall performance.
at Object.wrappedBinding (miniflare-internal:wrapped:setup-environment:14:34)
at requestHandler (/Users/jamalsoueidan/Development/booking-store/node_modules/@remix-run/server-runtime/dist/esm/server.js:136:303)
at /Users/jamalsoueidan/Development/booking-store/node_modules/@shopify/remix-oxygen/dist/development/index.js:103:28
at Object.fetch (/Users/jamalsoueidan/Development/booking-store/server.ts:97:24)
at withRequestHook (Users/jamalsoueidan/Development/booking-store/node_modules/@shopify/mini-oxygen/dist/vite/worker-entry.js:1053:20)
Request POST https://booking-shopify-api.azurewebsites.net/api/products/get-users-image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants