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

🐛 BUG: Infinite building with --x-dev-env and an app with a build #6876

Closed
zwily opened this issue Oct 1, 2024 · 23 comments · Fixed by #6881 or #6908
Closed

🐛 BUG: Infinite building with --x-dev-env and an app with a build #6876

zwily opened this issue Oct 1, 2024 · 23 comments · Fixed by #6881 or #6908
Labels
bug Something that isn't working

Comments

@zwily
Copy link
Contributor

zwily commented Oct 1, 2024

Which Cloudflare product(s) does this pertain to?

Wrangler

What version(s) of the tool(s) are you using?

3.79.0 [Wrangler]

What version of Node are you using?

20

What operating system and version are you using?

Man Sequoia

Describe the Bug

Observed behavior

When I have a build process (remix, in this case), static assets on, and use wrangler dev --x-dev-env, wrangler gets into an infinite loop of rebuilding my app.

Clone github.com:zwily/remix-wrangler-much-restarting and run npm i && npm run dev. I also tested this with wrangler HEAD, which includes --x-dev-env on by default, and it still happens.

If I comment out build.command from wrangler.toml it doesn't happen. Should I just make sure I always build before deploy and remove that line?

Please provide a link to a minimal reproduction

https://github.com/zwily/remix-wrangler-much-restarting

Please provide any relevant error logs

No response

@zwily zwily added the bug Something that isn't working label Oct 1, 2024
@threepointone
Copy link
Contributor

@RamIdeas

@RamIdeas
Copy link
Contributor

RamIdeas commented Oct 2, 2024

Taking a look now

cc @penalosa

@RamIdeas
Copy link
Contributor

RamIdeas commented Oct 2, 2024

Got a fix going here #6881

Notably, you'll also need to set build.watch_dir = ./app

@zwily
Copy link
Contributor Author

zwily commented Oct 2, 2024

Got a fix going here #6881

Notably, you'll also need to set build.watch_dir = ./app

We'll need that even when your PR is merged?

@RamIdeas
Copy link
Contributor

RamIdeas commented Oct 2, 2024

Got a fix going here #6881
Notably, you'll also need to set build.watch_dir = ./app

We'll need that even when your PR is merged?

Yeah fixing the bug made me think I broke something else. But the default build.watch_dir is ./src, so once I prevented the custom build infinite loop, changes to ./app/routes/_index.tsx didn't run the custom build until I added:

[build]
command = "npm run build"
+ watch_dir = "./app"

@threepointone
Copy link
Contributor

@RamIdeas That seems like a bug too, since that file is included in the worker bundle and should trigger a worker reload anyway, even without setting watch_dir. Does this problem exist even without --x-dev-env?

@threepointone
Copy link
Contributor

@zwily looking at youre repo, why are you calling remix build as a custom build at all? npm run dev is running remix in manual mode, and rebuilds the app anway.

@zwily
Copy link
Contributor Author

zwily commented Oct 3, 2024 via email

@threepointone
Copy link
Contributor

@RamIdeas Does this problem exist even without --x-dev-env?

@RamIdeas
Copy link
Contributor

RamIdeas commented Oct 4, 2024

The e2e test runs with the flag disabled and enabled which confirms it does not. The bug was caused by explicitly retriggering the whole flow in –x-dev-env so makes sense it did not appear in the old flow

@threepointone
Copy link
Contributor

threepointone commented Oct 4, 2024

I should clarify. We shouldn't have to put ./app as watch_dir, because we're using remix dev which generates the file that's fed into wrangler dev, and should retrigger a rebuild anyway. That works fine with without --x-dev-env, even with a custom build, and even if we're watching another directory. Can you explain to me what the bug with --x-dev-env was? And whether you need to add ./app as watch_dir now? (because you ideally shouldn't)

@RamIdeas
Copy link
Contributor

RamIdeas commented Oct 4, 2024

I think I know what you mean. If this works in the old flow, can you explain how? From my understanding, the wranger.toml has main = "./build/index.js" which wrangler will watch. But what causes that file to be updated, i.e what causes the custom build to run, when files in ./app change?

@threepointone
Copy link
Contributor

remix dev generates ./build/index.js anytime anything in the ./app import chain changes

@RamIdeas
Copy link
Contributor

RamIdeas commented Oct 4, 2024

But remix dev wasn't mentioned here. If though, wrangler dev was running without a custom build and remix dev was running in a separate terminal, then it would just work™️

@threepointone
Copy link
Contributor

In the repro https://github.com/zwily/remix-wrangler-much-restarting/blob/2490e3c018d357d1be57270018f12926d4990980/package.json#L9

Here remix is running in manual mode remix dev --manual -c "wrangler dev"

@threepointone
Copy link
Contributor

My question, setting watch_dir = ./app shouldn't be required in this usecase, so is it so even with --x-dev-env / after your fix?

@RamIdeas
Copy link
Contributor

RamIdeas commented Oct 4, 2024

My question, setting watch_dir = ./app shouldn't be required in this usecase, so is it so even with --x-dev-env / after your fix?

Yeah good catch. watch_dir is not required if using remix dev, only if using remix build in a custom build

@threepointone
Copy link
Contributor

ok dope, thank you very much for clarifying!

I think we need to fix the template and remove npm run build as the custom build step in the remix template, that's not required either

@zwily
Copy link
Contributor Author

zwily commented Oct 4, 2024

I must be doing something wrong because this is still infinite building with 3.80.0 on this app: https://github.com/zwily/remix-wrangler-much-restarting

I updated it for wrangler 3.80.0, removed the custom build and watch dir, and now when I npm run dev and then save something in app/ I get this:

➜ ~/Source/my-remix-app2 (main|✚3) $ npm run dev

> dev
> remix dev --manual -c "npm start"


 💿  remix dev

 info  building...
 info  built (314ms)

> start
> wrangler dev ./build/index.js


 ⛅️ wrangler 3.80.0
-------------------

[wrangler:inf] Ready on http://localhost:8787
⎔ Starting local server...
[REMIX DEV] 62100d3d ready

[wrangler:inf] GET / 200 OK (13ms)
[wrangler:inf] GET / 200 OK (9ms)
[wrangler:inf] GET / 200 OK (7ms)
 info  rebuilding... (~ app/routes/_index.tsx)
⎔ Reloading local server...
 info  rebuilt (119ms)
[REMIX DEV] 62100d3d ready
 info  app server ready (121ms)
 info  hmr

⎔ Reloading local server...
[REMIX DEV] 62100d3d ready
⎔ Reloading local server...
[REMIX DEV] 62100d3d ready
⎔ Reloading local server...
[REMIX DEV] 62100d3d ready
⎔ Reloading local server...
[REMIX DEV] 62100d3d ready
..........forever...........

@zwily
Copy link
Contributor Author

zwily commented Oct 4, 2024

Also, I just pushed an update to it that makes it match my proposed update for the official remix template classic-remix-compiler/cloudflare-workers. You can add --no-x-dev-env to the start command in package.json and verify that without x-dev-env, it doesn't restart infinitely, and with it it does.

zwily added a commit to zwily/remix that referenced this issue Oct 4, 2024
There's an ongoing issue with multiple reloads with the new x-dev-env,
so we're turning it off for now:

cloudflare/workers-sdk#6876
zwily added a commit to zwily/remix that referenced this issue Oct 4, 2024
There's an ongoing issue with multiple reloads with the new x-dev-env,
so we're turning it off for now:

cloudflare/workers-sdk#6876
@threepointone threepointone reopened this Oct 4, 2024
@zwily
Copy link
Contributor Author

zwily commented Oct 4, 2024

I've noticed another issue too, which I suspect is unrelated, but similar. Using the same sample repo (but adding --no-x-dev-env to avoid the infinite reloading), do npm run dev and then watch your console while saving app/routes/_index.tsx. You'll notice that as you save it more and more times, you see the server reloading more and more. So on the first save I get this output:

 info  rebuilding... (~ app/routes/_index.tsx)
⎔ Reloading local server...
⎔ Reloading local server...
[REMIX DEV] e7efd9e1 ready

 info  app server ready (226ms)
 info  hmr
⎔ Reloading local server...
[REMIX DEV] e7efd9e1 ready
[REMIX DEV] e7efd9e1 ready

On the 10th save:

 info  rebuilding... (~ app/routes/_index.tsx)
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
[REMIX DEV] e7efd9e1 ready

 info  app server ready (329ms)
 info  hmr
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
[REMIX DEV] e7efd9e1 ready
[REMIX DEV] e7efd9e1 ready

This has been happening for awhile and I've gotten used to just restarting the process periodically, but can you think of what might be causing this? Should I open a new ticket? Possible it's remix's fault? Or maybe this is fixed in the new dev env?

@threepointone
Copy link
Contributor

threepointone commented Oct 5, 2024

I can confirm that infinite reloads happen with --x-dev-env on, and doesn't without, on a standard remix app (I'm using https://github.com/threepointone/remix-workers-app)

@zwily
Copy link
Contributor Author

zwily commented Oct 7, 2024

This fix seems to work. And my last comment about the server reloads increasing every time a file is saved also seems to be addressed in --x-dev-env.

Thank you!

MichaelDeBoey pushed a commit to zwily/remix that referenced this issue Oct 9, 2024
There's an ongoing issue with multiple reloads with the new x-dev-env,
so we're turning it off for now:

cloudflare/workers-sdk#6876
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
None yet
3 participants