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

Dev mode is currently broken #2

Closed
kinngh opened this issue Apr 17, 2022 · 6 comments
Closed

Dev mode is currently broken #2

kinngh opened this issue Apr 17, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@kinngh
Copy link
Owner

kinngh commented Apr 17, 2022

The npm run dev is currently broken leading to a lot of 4xx and 5xx errors since the dev mode implementation is half baked. The production mode runs just fine with npm run build followed by a npm run start. HMR isn't supported in production mode, so any changes to React side of things need a npm run build and refresh in the embedded app to update.

@kinngh kinngh added the bug Something isn't working label Apr 17, 2022
@ivorpad
Copy link

ivorpad commented Apr 17, 2022

A couple of issues:

  1. I suggest changing isTest to isDev and avoid using it like !isDev as it is confusing. Instead:

if(isDev) // do dev things

If you have isDev then isProd is not necessary — or the other way around.

  1. Calling the middlewares method in vite throws an error because vite in LOC 114 is undefined, you're not returning in server/index.js line 98.

Should be:

vite = await import("vite").then(({ createServer }) =>
  createServer({
    root,
    logLevel: isDevelopment ? "error" : "info",
    server: {
      port: PORT,
      hmr: {
        protocol: "ws",
        host: "localhost",
        port: 64999,
        clientPort: 64999,
      },
      middlewareMode: "html",
    },
  })
);

() => {} // undefined vs () => ({}) // {}

@kinngh
Copy link
Owner Author

kinngh commented Apr 17, 2022

After reading some documentation, npm run dev is now working again. Also thanks to your fix for vite!

Commit 1fde536 temporarily fixes the issue. I am currently taking a look at the whole isProd / isTest blunder that creeped in.

Anyways, thank you for bringing this to my notice and suggesting changes, @ivorpad !

@kinngh kinngh closed this as completed Apr 17, 2022
@kinngh
Copy link
Owner Author

kinngh commented Apr 17, 2022

Update: fbd0f8d fixes issues created in 1fde536 and implements isDev instead of multi-variable mess.

Again, thank you for bringing this up!

@patrickbolle
Copy link

@kinngh Do you know how we could get this working with Shopify's HMR fixes for Safari/Firefox?

They seem to have fixed it in combination with their newest CLI upgrade and updated docs here: Shopify/shopify-app-template-node#984

I've tried putting in their hmrConfig code into your template, but it doesn't work - no front-end changes are shown until reloading page.

@kinngh
Copy link
Owner Author

kinngh commented Aug 19, 2022

@patrickbolle let me take a look at this. I'm currently down with a terrible viral fever, so it could be another few days before I'm back on track^

@kinngh
Copy link
Owner Author

kinngh commented Aug 20, 2022

@patrickbolle transferring this to a separate issue #22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants