vite icon indicating copy to clipboard operation
vite copied to clipboard

fix: prevent unnecessary reloads

Open sibbng opened this issue 2 years ago • 3 comments

Description

I'm working on a Tailwind project, when I update an html file that is tracked by Tailwind, Vite full-reloads all pages.

If files are tracked by Tailwind they bypass this check: https://github.com/vitejs/vite/blob/a9ccedd18a20b2fb5e15df191f7d4f1ff464e04b/packages/vite/src/node/server/hmr.ts#L105

In my case, hmrContext.modules is:

hmrContext.modules
[
  ModuleNode {
    id: 'C:/Users/sib/dev/luidev/components/HTML/default.html',  
    file: 'C:/Users/sib/dev/luidev/components/HTML/default.html',
    importers: Set(1) {
      ModuleNode {
        id: 'C:/Users/sib/dev/luidev/server/style.css',
        file: 'C:/Users/sib/dev/luidev/server/style.css',        
        importers: [Set],
        importedModules: [Set],
        acceptedHmrDeps: Set(0) {},
        transformResult: [Object],
        ssrTransformResult: null,
        ssrModule: null,
        ssrError: null,
        lastHMRTimestamp: 1653670227215,
        lastInvalidationTimestamp: 0,
        url: '/style.css',
        type: 'js',
        isSelfAccepting: true
      }
    },
    importedModules: Set(0) {},
    acceptedHmrDeps: Set(0) {},
    transformResult: null,
    ssrTransformResult: null,
    ssrModule: null,
    ssrError: null,
    lastHMRTimestamp: 0,
    lastInvalidationTimestamp: 1653670238242,
    url: '/@fs/C:/Users/sib/dev/luidev/components/HTML/default.html',
    type: 'js',
    isSelfAccepting: false
  }
]

When they bypass here, they fall into here: https://github.com/vitejs/vite/blob/a9ccedd18a20b2fb5e15df191f7d4f1ff464e04b/packages/vite/src/node/server/hmr.ts#L164-L171

Which is the source of problem. This part of code doesn't send path to full-reload events and this cause full reload on all pages:

https://github.com/vitejs/vite/blob/a9ccedd18a20b2fb5e15df191f7d4f1ff464e04b/packages/vite/src/client/client.ts#L116-L131

I assume this is what happened to @cdauth, as he described in #6695:

form state to be lost whenever I edit some unrelated file

Additional context


What is the purpose of this pull request?

  • [x] Bug fix
  • [ ] New Feature
  • [ ] Documentation update
  • [ ] Other

Before submitting the PR, please make sure you do the following

  • [x] Read the Contributing Guidelines.
  • [x] Read the Pull Request Guidelines and follow the Commit Convention.
  • [x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • [x] Ideally, include relevant tests that fail without this PR but pass with it.

sibbng avatar May 27 '22 17:05 sibbng

I guess changing

  if (!hmrContext.modules.length) {
    // html file cannot be hot updated
    if (file.endsWith('.html')) {
      config.logger.info(colors.green(`page reload `) + colors.dim(shortFile), {
        clear: true,
        timestamp: true
      })
      ws.send({
        type: 'full-reload',
        path: config.server.middlewareMode
          ? '*'
          : '/' + normalizePath(path.relative(config.root, file))
      })
    } else {
      // loaded but not in the module graph, probably not js
      debugHmr(`[no modules matched] ${colors.dim(shortFile)}`)
    }
    return
  }

to

  // html file cannot be hot updated
  if (file.endsWith('.html')) {
    config.logger.info(colors.green(`page reload `) + colors.dim(shortFile), {
      clear: true,
      timestamp: true
    })
    ws.send({
      type: 'full-reload',
      path: config.server.middlewareMode
        ? '*'
        : '/' + normalizePath(path.relative(config.root, file))
    })
    return
  }

  if (!hmrContext.modules.length) {
    // loaded but not in the module graph, probably not js
    debugHmr(`[no modules matched] ${colors.dim(shortFile)}`)
    return
  }

is better. (I'm not sure if I'm correct.)

sapphi-red avatar Jun 16 '22 06:06 sapphi-red

I'm running into a slightly different issue, but the fix is the same. Is there a good reason not to add the path that caused the full-reload to the 'full-reload' event payload? In my case, I'm experimenting with vite-node and would like to know the path that caused the full reload to construct a nice log message.

0xOlias avatar Oct 31 '23 04:10 0xOlias

Regarding sapphi's comment, I suppose if the HTML was ever in the hmrContext.modules, it may be safer to invalidate the modules anyway to prevent potential issues with tailwind.

With the updated understanding over the last two years, I agree with that. 😅

sapphi-red avatar May 18 '24 07:05 sapphi-red