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

Hot reloading reloads the whole page #1704

Closed
drager opened this issue Jun 30, 2022 · 12 comments · Fixed by #1705
Closed

Hot reloading reloads the whole page #1704

drager opened this issue Jun 30, 2022 · 12 comments · Fixed by #1705

Comments

@drager
Copy link

drager commented Jun 30, 2022

What is the current behaviour?
When changing e.g. text in a component the whole page is reloaded and not just only the component.

Steps to Reproduce
Steps to reproduce the behavior:

  1. Generate a new project with npx preact-cli create typescript my-app
  2. Run yarn dev
  3. Change text in a component and the whole page is reloaded

(I also updated all the packages to it's latest versions). Also tried preact watch --refresh without luck.

What is the expected behaviour?
I expect only the changed component to be reloaded and the rest of the page state is kept.

Please mention any other relevant information
tsconfig.json

{
    "compilerOptions": {
        /* Basic Options */
        "target": "ES5",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', or 'ESNEXT'. */
        "module": "ESNext",                       /* Specify module code generation: 'none', commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */
        // "lib": [],                             /* Specify library files to be included in the compilation:  */
        "allowJs": true,                          /* Allow javascript files to be compiled. */
        // "checkJs": true,                       /* Report errors in .js files. */
          "jsx": "react",                           /* Specify JSX code generation: 'preserve', 'react-native', or 'react'. */
        "jsxFactory": "h",                        /* Specify the JSX factory function to use when targeting react JSX emit, e.g. React.createElement or h. */
        "jsxFragmentFactory": "Fragment",         /* Specify the JSX fragment factory function to use when targeting react JSX emit with jsxFactory compiler option is specified, e.g. Fragment. */
        // "declaration": true,                   /* Generates corresponding '.d.ts' file. */
        // "sourceMap": true,                     /* Generates corresponding '.map' file. */
        // "outFile": "./",                       /* Concatenate and emit output to single file. */
        // "outDir": "./",                        /* Redirect output structure to the directory. */
        // "rootDir": "./",                       /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
        // "removeComments": true,                /* Do not emit comments to output. */
        "noEmit": true,                           /* Do not emit outputs. */
        // "importHelpers": true,                 /* Import emit helpers from 'tslib'. */
        // "downlevelIteration": true,            /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */
        // "isolatedModules": true,               /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */

        /* Strict Type-Checking Options */
        "strict": true,                           /* Enable all strict type-checking options. */
        // "noImplicitAny": true,                 /* Raise error on expressions and declarations with an implied 'any' type. */
        // "strictNullChecks": true,              /* Enable strict null checks. */
        // "noImplicitThis": true,                /* Raise error on 'this' expressions with an implied 'any' type. */
        // "alwaysStrict": true,                  /* Parse in strict mode and emit "use strict" for each source file. */

        /* Additional Checks */
        // "noUnusedLocals": true,                /* Report errors on unused locals. */
        // "noUnusedParameters": true,            /* Report errors on unused parameters. */
        // "noImplicitReturns": true,             /* Report error when not all code paths in function return a value. */
        // "noFallthroughCasesInSwitch": true,    /* Report errors for fallthrough cases in switch statement. */

        /* Module Resolution Options */
        "moduleResolution": "node",               /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */
        "esModuleInterop": true,                  /* */
        // "baseUrl": "./",                       /* Base directory to resolve non-absolute module names. */
        // "paths": {},                           /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */
        // "rootDirs": [],                        /* List of root folders whose combined content represents the structure of the project at runtime. */
        // "typeRoots": [],                       /* List of folders to include type definitions from. */
        // "types": [],                           /* Type declaration files to be included in compilation. */
        // "allowSyntheticDefaultImports": true,  /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */
        // "preserveSymlinks": true,              /* Do not resolve the real path of symlinks. */

        /* Source Map Options */
        // "sourceRoot": "./",                    /* Specify the location where debugger should locate TypeScript files instead of source locations. */
        // "mapRoot": "./",                       /* Specify the location where debugger should locate map files instead of generated locations. */
        // "inlineSourceMap": true,               /* Emit a single file with source maps instead of having a separate file. */
        // "inlineSources": true,                 /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */

        /* Experimental Options */
        // "experimentalDecorators": true,        /* Enables experimental support for ES7 decorators. */
        // "emitDecoratorMetadata": true,         /* Enables experimental support for emitting type metadata for decorators. */

        /* Advanced Options */
        "skipLibCheck": true,                      /* Skip type checking of declaration files. */
        "downlevelIteration": true,
        "baseUrl": "./",
        "paths": {
          "react": ["./node_modules/preact/compat/"],
          "react-dom": ["./node_modules/preact/compat/"]
        }
    },
    "include": ["src/**/*", "tests/**/*"]
}
  System:
    OS: Linux 5.4 NixOS 20.09 (Nightingale) 20.09.3987.35fc6e4a277 (Nightingale)
    CPU: (4) x64 Intel(R) Core(TM) i7-6500U CPU @ 2.50GHz
  Binaries:
    Node: 14.16.0 - /nix/store/nkp79spc3pxdflv4d4zs148vc26pc1hs-nodejs-14.16.0/bin/node
    Yarn: 1.22.5 - /nix/store/m3x1s38v3xmvcga4p10qjjpad6fpay82-yarn-1.22.5/bin/yarn
    npm: 6.14.11 - /nix/store/nkp79spc3pxdflv4d4zs148vc26pc1hs-nodejs-14.16.0/bin/npm
  Browsers:
    Firefox: 88.0
  npmPackages:
    preact: ^10.3.1 => 10.8.2 
    preact-cli: ^3.3.5 => 3.3.5 
    preact-render-to-string: ^5.1.4 => 5.2.0 
    preact-router: ^4.0.1 => 4.0.1 ```
@rschristian
Copy link
Member

Without the --refresh flag, HMR will not be enabled, so that part of your comment can be answered with "working as intended".

As for --refresh not working though, can confirm that I've been seeing that for a while.

@JoviDeCroock Any chance you might know what's going on here? As far as I can tell we're following Prefresh's instructions for Webpack. We're not using @prefresh/babel-plugin, but adding it (or replacing react-refresh/babel) seems to have no effect.

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jun 30, 2022

I can take a look, I haven't generally been noticing any issues with webpack 4 but could be that something got broken. We used to have a test for webpack 4 and webpack 5 but looks like I removed the 4 one 😅 will try and take a look this weekend

Generally though, you should be able to capture the error from here if you add a log in node_modules

@rschristian
Copy link
Member

rschristian commented Jun 30, 2022

Thanks! I'll keep poking at it, but HMR is very much magic that I have not yet delved into.

FWIW I'm not seeing any errors after adding a log, just full-page refresh on any change in a component. Might not be Prefresh in that case but a Webpack dev server thing?

Edit: Found that explicitly setting liveReload: false fixes this... weird that it's not automatically disabled if hot is set to true? Will get a PR together, looks totally unrelated to Prefresh I think.

@drager
Copy link
Author

drager commented Jul 1, 2022

Thanks for the quick responses!

We used to have a test for webpack 4 and webpack 5 but looks like I removed the 4 one sweat_smile will try and take a look this weekend

Should preact-cli upgrade to webpack 5 then?

@rschristian
Copy link
Member

Forgot to mention, you can fix this in your own preact.config.js until next release:

// preact.config.js
export default (config, env, helpers) => {
    config.devServer.liveReload = false;
}

Should preact-cli upgrade to webpack 5 then?

...that's not a trivial task. #1645 exists to do that, but Webpack 5 would be behind a major version for CLI and there's some other things I'd like to do as well. Likely a long way off, due to my own time limits.

@drager
Copy link
Author

drager commented Jul 1, 2022

Forgot to mention, you can fix this in your own preact.config.js until next release:

// preact.config.js
export default (config, env, helpers) => {
    config.devServer.liveReload = false;
}

Should preact-cli upgrade to webpack 5 then?

...that's not a trivial task. #1645 exists to do that, but Webpack 5 would be behind a major version for CLI and there's some other things I'd like to do as well. Likely a long way off, due to my own time limits.

Thanks a lot!

Ah yeah, I figured that. We'll see what happens in the future then 😊

@simonv3
Copy link

simonv3 commented Jan 9, 2023

We're still seeing this locally with preact-cli v3.4.3 as well as adding the config.devServer.liveReload = false; line to the config. Are there any next places for us to look to try and fix this?

@rschristian
Copy link
Member

rschristian commented Jan 9, 2023

We're still seeing this locally with preact-cli v3.4.3 as well as adding the config.devServer.liveReload = false; line to the config. Are there any next places for us to look to try and fix this?

I can't reproduce. Are you using the --refresh flag? HMR is not enabled without it.

Edit: To test, I've added a simple console.log('(header|body) rendered'); statement into both the header and home components. When I update the header, I only get the header console message. Same goes for the home component. Without further info, it seems to be working correctly.

@simonv3
Copy link

simonv3 commented Jan 9, 2023

Yeah, we're using the --refresh flag: preact watch --refresh.

When adding console.log(config) to preact.config.js this is what gets logged:

devServer: {
    hot: true,
    liveReload: false,
    compress: true,
    devMiddleware: { publicPath: '/', stats: 'errors-warnings' },
    static: { directory: '/home/simon/src/un/portal/src', watch: [Object] },
    https: undefined,
    port: 8080,
    host: '0.0.0.0',
    allowedHosts: 'all',
    historyApiFallback: true,
    client: { logging: 'none', overlay: false },
    proxy: undefined
  }

which looks correct as for what I've seen in the thread here? We are running a fairly old project, so I'm wondering if there's other places that might be affecting this? Deconstructing it to get to figure out where things are going wrong seems like a pretty big job right now so I'm just wondering if there's anything immediately that would be an obvious place to check.

@rschristian
Copy link
Member

Yeah, that looks like the vanilla config as is correct.

The only immediate thoughts I have is do you have a default export for your component in your entrypoint (i.e., like this) and does your app have some sort of root HTML element (i.e., like this)? Some people do their own mounting which screws some things up and without a root element I know hydration can get borked. Not sure if either would actually affect HMR, but they're the only things that immediately come to mind.

@simonv3
Copy link

simonv3 commented Jan 9, 2023

Yeah we do have those two things, but we also have a production only build process that separately compiles some of our components into stand-alone widgets. I wonder if that might be having an impact. I'll try to dig into that. Thanks for the fast response!

@rschristian
Copy link
Member

Hm, if it's a prod-only build process, I don't imagine it'll be the cause of dev HMR issues?

Sorry I can't be more help, but if you are able to provide a minimal repo I can take a look.

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 a pull request may close this issue.

4 participants