fix: prevent unnecessary reloads by sibbng · Pull Request #8356 · vitejs/vite

sibbng · 2022-05-27T17:10:34Z

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:

vite/packages/vite/src/node/server/hmr.ts

Line 105 in a9ccedd

if (!hmrContext.modules.length) {

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:

vite/packages/vite/src/node/server/hmr.ts

Lines 164 to 171 in a9ccedd

    
           if (needFullReload) { 
        
             config.logger.info(colors.green(`page reload `) + colors.dim(file), { 
        
               clear: true, 
        
               timestamp: true 
        
             }) 
        
             ws.send({ 
        
               type: 'full-reload' 
        
             })

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:

vite/packages/vite/src/client/client.ts

Lines 116 to 131 in a9ccedd

    
           if (payload.path && payload.path.endsWith('.html')) { 
        
             // if html file is edited, only reload the page if the browser is 
        
             // currently on that page. 
        
             const pagePath = decodeURI(location.pathname) 
        
             const payloadPath = base + payload.path.slice(1) 
        
             if ( 
        
               pagePath === payloadPath || 
        
               payload.path === '/index.html' || 
        
               (pagePath.endsWith('/') && pagePath + 'index.html' === payloadPath) 
        
             ) { 
        
               location.reload() 
        
             } 
        
             return 
        
           } else { 
        
             location.reload() 
        
           }

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?

Bug fix
New Feature
Documentation update
Other

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

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

sapphi-red · 2022-06-16T06:53:54Z

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.)

typedarray · 2023-10-31T04:11:02Z

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.

bluwy

I think the current PR is fine by me, but probably be safer if we do it like this too:

vite/packages/vite/src/node/server/hmr.ts

Lines 203 to 208 in c3b3d3c

    
           hot.send({ 
        
             type: 'full-reload', 
        
             path: config.server.middlewareMode 
        
               ? '*' 
        
               : '/' + normalizePath(path.relative(config.root, file)), 
        
           })

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.

sapphi-red · 2024-05-18T07:43:48Z

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. 😅

patak-dev · 2025-02-07T08:51:56Z

I think we should close this one as stale, @sibbng @philipp-spiess if this is still an issue in Tailwinds v4, would you create an issue so we can properly track this? And you can send a new PR to continue this discussion against it in that case.

sibbng added 2 commits May 27, 2022 19:21

fix: prevent unnecessary reloads

c0e78d6

chore: format

eab0a28

sapphi-red added the feat: hmr label May 28, 2022

sapphi-red added the p2-nice-to-have Not breaking anything but nice to have (priority) label Jun 16, 2022

bluwy reviewed May 18, 2024

View reviewed changes

patak-dev closed this Feb 7, 2025

sibbng deleted the fix/html-full-reload branch February 8, 2025 11:30

sapphi-red mentioned this pull request Jul 10, 2025

fix(hmr): register css deps as type: asset #20391

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

fix: prevent unnecessary reloads#8356

fix: prevent unnecessary reloads#8356
sibbng wants to merge 2 commits intovitejs:mainfrom
sibbng:fix/html-full-reload

sibbng commented May 27, 2022 •

edited

Loading

Uh oh!

sapphi-red commented Jun 16, 2022

Uh oh!

typedarray commented Oct 31, 2023

Uh oh!

bluwy left a comment

Uh oh!

sapphi-red commented May 18, 2024

Uh oh!

patak-dev commented Feb 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	if (needFullReload) {
	config.logger.info(colors.green(`page reload `) + colors.dim(file), {
	clear: true,
	timestamp: true
	})
	ws.send({
	type: 'full-reload'
	})

	if (payload.path && payload.path.endsWith('.html')) {
	// if html file is edited, only reload the page if the browser is
	// currently on that page.
	const pagePath = decodeURI(location.pathname)
	const payloadPath = base + payload.path.slice(1)
	if (
	pagePath === payloadPath \|\|
	payload.path === '/index.html' \|\|
	(pagePath.endsWith('/') && pagePath + 'index.html' === payloadPath)
	) {
	location.reload()
	}
	return
	} else {
	location.reload()
	}

	hot.send({
	type: 'full-reload',
	path: config.server.middlewareMode
	? '*'
	: '/' + normalizePath(path.relative(config.root, file)),
	})

Uh oh!

Comments

Conversation

sibbng commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context

What is the purpose of this pull request?

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

Uh oh!

sapphi-red commented Jun 16, 2022

Uh oh!

typedarray commented Oct 31, 2023

Uh oh!

bluwy left a comment

Choose a reason for hiding this comment

Uh oh!

sapphi-red commented May 18, 2024

Uh oh!

patak-dev commented Feb 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sibbng commented May 27, 2022 •

edited

Loading