[v2] (wails:dev) smart rebuild

[ironpark]

Currently, the wails dev command rebuilds both the frontend and the backend even if the backend code changes that do not affect the frontend.

When the frontend's code base is light, it's not a big problem, but if the frontend build time exceeds a few minutes, it feels like a big problem in productivity.

How about conditionally skipping the frontend build by checking for changed files or checking if the generated "Wails JS" has changed?

[leaanthony]

Agreed! Happy to accept a PR for it!

[leaanthony]

PR to test: https://github.com/wailsapp/wails/pull/1071

[ironpark]

@leaanthony

In the last few days, after reviewing #1071 PR and looking at the source code related to the dev command, I found some areas for improvement.

• watch ignore It is hardcoded to omit the node_modules and build directories in the process of setting the directory to watch inside the initialiseWatcher function. It seems to be a better design to change this part so that it can be set as a flag.

• add ignore .git directory It seems like a good idea to ignore the .git directory by default, like node_modules .

• recv watcher.Errors channel stream In our current implementation, we are only passing Events from watcher and not reading Errors . If messages accumulate in the Errors channel, watcher 's loop may hang and it may cause a potential bug that causes the build to stop.

• go file ast check By using the default go/parser package, when the go file is changed, you can parse the file and check whether the build is possible by checking for errors. This prevents build from being triggered too often while editing the source code.

[leaanthony]

Thanks for the feedback! I'll answer each point:

• watch ignore It is hardcoded to omit the node_modules and build directories in the process of setting the directory to watch inside the initialiseWatcher function. It seems to be a better design to change this part so that it can be set as a flag.

Yeah, we could add this to the config.

• add ignore .git directory It seems like a good idea to ignore the .git directory by default, like node_modules .

If we did the above config we could add this by default

• recv watcher.Errors channel stream In our current implementation, we are only passing Events from watcher and not reading Errors . If messages accumulate in the Errors channel, watcher 's loop may hang and it may cause a potential bug that causes the build to stop.

Can you please provide more details on this - thanks!

• go file ast check By using the default go/parser package, when the go file is changed, you can parse the file and check whether the build is possible by checking for errors. This prevents build from being triggered too often while editing the source code.

If you want to code this, I'd be happy to merge it. It's not a small task!

[ironpark]

• Can you please provide more details on this - thanks! dev.go doWatcherLoop function

for quit == false { // Line : 536
	select {
	case exitCode := <-exitCodeChannel:
                // ...
	case item := <-watcher.Events:
		// ...
	case <-timer.C:
		// ...
	case <-quitChannel:
		// ...
	}
}

need to add code that reads and outputs the error channel.

for quit == false { // Line : 536
	select {
	case exitCode := <-exitCodeChannel:
                // ...
++	case err := <-watcher.Errors:
++	 	LogDarkYellow(err.Error())
	case item := <-watcher.Events:
		// ...
	case <-timer.C:
		// ...
	case <-quitChannel:
		// ...
	}
}

[leaanthony]

Regarding ignore dirs, we have 2 scenarios: "start with", eg build and "contains", eg "node_modules" because it could be far down the path. Do you think 2 flags are needed or is there a better way?

[leaanthony]

I'm ok pushing this out to v2.1 and seeing if we need it then.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ironpark]

Regarding ignore dirs, we have 2 scenarios: "start with", eg build and "contains", eg "node_modules" because it could be far down the path. Do you think 2 flags are needed or is there a better way?

An approach like .gitignore is best in my opinion. saving in a file format such as .wailsignore or .watchignore is easy to modify.

It is convenient to be able to use wildcards to exclude directories as well as certain types of filenames.

For example, a test file like *_test.go excluding this can also reduce unnecessary builds.

[leaanthony]

Hey @Ironpark! Good to see you! I actually really like this idea, but I'm wondering if we can adopt the same idea but put it in wails.json to avoid multiple configs? Something like:

"dev:ignore": "frontend/my/dir/*.js, generated/*.go",

Thoughts?

[ironpark]

Good to see you too! :D Writing on a single line using , as a separator is prone to human error. I think writing in an array format like the one below reduces mistakes and is more readable.

"dev:ignore": [
    "frontend/my/dir/*.js",
    "generated/*.go",
    "*_test.go"
],

[leaanthony]

Of course 👍 Great idea.

[ironpark]

@leaanthony I came up with a more concrete approach to the AST-based file inspection I proposed last January, and wrote some POC code.

package main

import (
	"bytes"
	"fmt"
	"go/ast"
	"go/format"
	"go/parser"
	"go/token"
	"golang.org/x/tools/go/ast/astutil"
	"hash/fnv"
	"strings"
)

func fileHashWithAST(path string) (uint64, error) {
	fset := token.NewFileSet()
	node, err := parser.ParseFile(fset, path, nil, 0)
	if err != nil {
		return 0, err
	}
	badAst := false
	pre := func(c *astutil.Cursor) bool {
		switch c.Node().(type) {
		case *ast.ImportSpec:
			c.Delete()
		case *ast.BadExpr, *ast.BadDecl, *ast.BadStmt:
			badAst = true
			return false
		}
		return true
	}
	astutil.Apply(node, pre, nil)
	if badAst {
		return 0, fmt.Errorf("bad ast")
	}
	buf := bytes.Buffer{}
	if err := format.Node(&buf, fset, node); err != nil {
		return 0, err
	}
	cleanStr := strings.Replace(buf.String(), " ", "", -1)
	cleanStr = strings.Replace(cleanStr, "\t", "", -1)
	cleanStr = strings.Replace(cleanStr, "\r\n", "", -1)
	cleanStr = strings.Replace(cleanStr, "\n", "", -1)
	hash := fnv.New64a()
	_, _ = hash.Write([]byte(cleanStr))
	return hash.Sum64(), nil
}

func main() {
	fmt.Println(fileHashWithAST("main.go"))
}

The basic idea is this

1. analyze the AST of the file
2. remove import statements, comments
3. remove all whitespace and newline characters and hash them.

This hash value is unchanged by comments, newlines, and import statements, so you can check for substantive logic changes.

When used in a file watcher

// Parsing failed: If the file is still being written, 
// a syntax error is assumed to have occurred due to an unfinished write.
if current_hash,err := ast_hash(file);err == nil{
    // Trigger a rebuild only if the hash value is different from before.
    if current_hash != before_hash[file] {
        before_hash[file] = current_hash
	// -> build update triggering
    }
}

[leaanthony]

That's quite impressive! Does it have any benefits over running the file through an md5 hasher? Incidentally, this is how task does file watching.

[ironpark]

That's quite impressive! Does it have any benefits over running the file through an md5 hasher? Incidentally, this is how task does file watching.

Fnv is a non-cryptographic hash function. It has no security requirements, so its implementation is simple and fast. I thought it was optimal for simply detecting changes to files as it is now.

Below is a microbenchmark. It's not very reliable due to the low number of iterations, but it's a good starting point.

BenchmarkFNV32-20     	 5614111	       194.5 ns/op
BenchmarkFNV64a-20    	 6161049	       198.0 ns/op
BenchmarkFNV128-20    	 5501001	       201.5 ns/op
BenchmarkMD5-20       	 4314483	       279.2 ns/op
BenchmarkSHA1-20      	 5616726	       212.1 ns/op
BenchmarkSHA224-20    	 5642409	       216.2 ns/op
BenchmarkSHA256-20    	 5274843	       227.8 ns/op
BenchmarkSHA512-20    	 1779057	       677.8 ns/op

[ironpark]

I took a moment to read the documentation. It looks like there are roughly 2-3 ways to use it with Task runner.

1. Run the watch process in the wails cli as it is now and trigger the task script
   • This means that the watch function of the task is not used.
2. Create a script or util that generates a checksum based on ast-hash and insert it into the execution condition of the task.
3. Almost like the second method, but using '&&' to make the task ignore it when it runs
   • wails check {{TASK}} && go build ......

[leaanthony]

The reason I was asking is because we have a similar technique for monitoring package.json for updates. We run it through md5 and check it against package.md5

[ironpark]

Background

In fact, MD5 was invented as a cryptographic hash function. However, several security flaws have been found that can quickly discover hash collisions, so it is not recommended for security purposes.

However, it is still a hash function, so it's not bad for verifying file checksums, and its speed is not bad either.

But if you're considering a non-secure hash algorithm, FNV-1a is a good choice. It's fast with a sufficiently good hash distribution and collision resistance because it is implemented in the Go standard hash package.

If you don't care about standard packages, I'm not sure if there are properly implemented packages, but Murmur2/3 is also a good choice. It is faster for hashing large data due to structural advantages and has a slightly better hash distribution.

Personal Views

Given all of the above, if you're hashing a single package.json file, there's honestly no big deal to use MD5 or not - it's still useful in that use case and provides plenty of speed.

For more specific information on this topic, we recommend the following links which-hashing-algorithm-is-best-for-uniqueness-and-speed

[leaanthony]

I think we'll look for an existing library for rebuilding the app in v3. Vite already handles HMR for the frontend so we're only really dealing with the Go code. I imagine we'll get the rebuild trigger to run a task by default.