I used the following command: <div class="highlight highlight-source-shell notrans

<a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/us

Thanks <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-u

<a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/us

Great find, and nice write up. From reading the <code class="notrans

cmd: templ generate with watch can generate corrupted Go files, because of race condition with fsnotify WRITE events,about a-h/templ

stefanohrmann commented on September 27, 2024 1

@a-h You are right about the dedup and debounce stuff. I didn't want to change the terminology of fsnotify and I marked the wrong place for the fix. I will correct it.

from templ.

joerdav commented on September 27, 2024

Thanks @stefanohrmann for the thorough bug report, and your time spent investigating. I've marked this as NeedsInvestigation because I think it could be worth looking into the viability of a fix which doesn't involve removing any commands. Such as ignoring events where the file is empty.

from templ.

stefanohrmann commented on September 27, 2024

@joerdav You are right and I think it is totally possible to fix this issue in the current implementation if you apply the dedup stuff from fsnotify here:

templ/cmd/templ/generatecmd/watcher/watch.go

Lines 73 to 99 in 753ba39

    
           func (w *RecursiveWatcher) loop() { 
        
           	for { 
        
           		select { 
        
           		case <-w.ctx.Done(): 
        
           			return 
        
           		case event, ok := <-w.w.Events: 
        
           			if !ok { 
        
           				return 
        
           			} 
        
           			if event.Has(fsnotify.Create) { 
        
           				if err := w.Add(event.Name); err != nil { 
        
           					w.Errors <- err 
        
           				} 
        
           			} 
        
           			// Only notify on templ related files. 
        
           			if !shouldIncludeFile(event.Name) { 
        
           				continue 
        
           			} 
        
           			w.Events <- event 
        
           		case err, ok := <-w.w.Errors: 
        
           			if !ok { 
        
           				return 
        
           			} 
        
           			w.Errors <- err 
        
           		} 
        
           	} 
        
           }

And with my suggestion i didn't want to remove any subcommands. I wanted to add a dedicated one for watching. But it is ok if this doesn't fit in your structure.

Edit note: Changed the link to right place for the fix.

from templ.

joerdav commented on September 27, 2024

Ah, not to dismiss your idea, just wanted to explore all options first, and I think minimal change to the outside API would be preferable.

from templ.

a-h commented on September 27, 2024

Great find, and nice write up.

From reading the fsnotify dedup function, it looks like it's more of a debounce than a deduplication, because it doesn't look at the message content (e.g. by having a windowing function or ring buffer of previous messages). It just waits for a timer to elapse before allowing the processing.

I think the logic to handle either a true deduplication (i.e. within a window of 100ms, if there is more than one operation of the same time, discard the earlier ones, and emit the latest), or debounce would go here:

templ/cmd/templ/generatecmd/watcher/watch.go

Lines 73 to 99 in 753ba39

    
           func (w *RecursiveWatcher) loop() { 
        
           	for { 
        
           		select { 
        
           		case <-w.ctx.Done(): 
        
           			return 
        
           		case event, ok := <-w.w.Events: 
        
           			if !ok { 
        
           				return 
        
           			} 
        
           			if event.Has(fsnotify.Create) { 
        
           				if err := w.Add(event.Name); err != nil { 
        
           					w.Errors <- err 
        
           				} 
        
           			} 
        
           			// Only notify on templ related files. 
        
           			if !shouldIncludeFile(event.Name) { 
        
           				continue 
        
           			} 
        
           			w.Events <- event 
        
           		case err, ok := <-w.w.Errors: 
        
           			if !ok { 
        
           				return 
        
           			} 
        
           			w.Errors <- err 
        
           		} 
        
           	} 
        
           }

from templ.

joerdav commented on September 27, 2024

I've created a debounce implementation, for now I've put the window at 100ms I'm hoping that this will be enough.

from templ.

cmd: templ generate with watch can generate corrupted Go files, because of race condition with fsnotify WRITE events about templ HOT 6 CLOSED

Comments (6)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	func (w *RecursiveWatcher) loop() {
	for {
	select {
	case <-w.ctx.Done():
	return
	case event, ok := <-w.w.Events:
	if !ok {
	return
	}
	if event.Has(fsnotify.Create) {
	if err := w.Add(event.Name); err != nil {
	w.Errors <- err
	}
	}
	// Only notify on templ related files.
	if !shouldIncludeFile(event.Name) {
	continue
	}
	w.Events <- event
	case err, ok := <-w.w.Errors:
	if !ok {
	return
	}
	w.Errors <- err
	}
	}
	}