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

cmd mistakenly running on TS Files, not input file only #532

Closed
c9845 opened this issue Sep 15, 2022 · 9 comments
Closed

cmd mistakenly running on TS Files, not input file only #532

c9845 opened this issue Sep 15, 2022 · 9 comments

Comments

@c9845
Copy link

c9845 commented Sep 15, 2022

My directory structure is a flat tree with source .ts files. These .ts files get combined into script.js by tsc. My goal is to have minify watch the script.js file and minify it to script.min.js in the same directory.

When I run minify -w -v -o website/static/js/script.min.js website/static/js/script.js, I get output like "copy website\static\js\common.ts to website\static\js\script.min.js" showing that minify is running on a change to the common.ts file. However, minify should only be running on changes to the script.js file per the command. To confirm minify is running on (and copying) .ts files, I can see that the script.js file contains the .ts file's contents for a brief time until tsc completes, minify notices the updated script.js, and the minifies it.

So my question is, why is minify running on a change to a .ts file if the command specifically notes it should run on changes to script.js?

Complete output is below for a single file change:

minify -w -v -o website/static/js/script.min.js website/static/js/script.js 
infer mimetype from file extensions
minify to output file website\static\js\script.min.js
(28.4291ms, 1.3 MB, 415 kB,  31.0%,  47 MB/s) - website\static\js\script.js to website\static\js\script.min.js
(18.5892ms, 415 kB, 415 kB, 100.0%,  22 MB/s) - website\static\js\script.min.js
copy website\static\js\common.ts to website\static\js\script.min.js
ERROR: cannot minify website\static\js\script.min.js: expected = instead of : in const statement on line 8 and column 17
    8: const apiBaseURL: string = "/api/";
                       ^
(  534.1µs,  10 kB,    0 B,   0.0%,  20 MB/s) - website\static\js\script.min.js
(25.3882ms, 1.3 MB, 415 kB,  31.0%,  53 MB/s) - website\static\js\script.js to website\static\js\script.min.js
(19.0289ms, 415 kB, 415 kB, 100.0%,  22 MB/s) - website\static\js\script.min.js
@c9845 c9845 changed the title cmd mistakenly tunning on TS Files, not input file only cmd mistakenly running on TS Files, not input file only Sep 16, 2022
@c9845
Copy link
Author

c9845 commented Sep 16, 2022

Note, this was with v2.12.1. With v2.3.6, this issue does not occur.

@c9845
Copy link
Author

c9845 commented Sep 17, 2022

Confirmed on Windows and macOS. Not sure what the "copy 'x' to 'y'" line is about yet.

@c9845
Copy link
Author

c9845 commented Sep 17, 2022

The issue seems to be caused by a task being created, and sent, for a file that should not be watched for changes.

  • A Task is being created for a change on a .ts file (note that any other file causes this, not just .ts files).
  • Task.sync is being set to true since NewTask() takes the inverse of the fileMatches() output.
  • This Task is then sent on the chanTasks channel and causing the "copy 'x' to 'y'" output as part of handling sync.

There are two questions I have:

  1. Why isn't fileMatches() being called before, and independently from, NewTask() and if false, calling break? This would prevent the Task from ever being created and sent.
  2. Why is the current implementation ignoring the --sync flag and setting Task.sync based on fileMatches()?

Original code (below line 480 in main.go):

//...
if !verbose {
    Info.Println(file, "changed")
}

task, err := NewTask(root, file, output, !fileMatches(file))
if err != nil {
    Error.Println(err)
    return 1
}
chanTasks <- task
//...

My proposed changes:

//...
if !verbose {
    Info.Println(file, "changed")
}

if !fileMatches(file) {
    break
}

task, err := NewTask(root, file, output, sync)
if err != nil {
    Error.Println(err)
    return 1
}
chanTasks <- task
//...

Implementing the code above does prevent the "copy 'x' to 'y'" output logging and the .ts file that is modified is not copied to the minify output file (both expected). I do, however, still see two logging outputs which I do not yet understand:

  • (6.959853ms, 189 kB, 60 kB, 31.7%, 27 MB/s) - website/static/js/script.js to website/static/js/script.min.js
  • (6.591812ms, 60 kB, 60 kB, 100.0%, 9.1 MB/s) - website/static/js/script.min.js

@c9845
Copy link
Author

c9845 commented Sep 17, 2022

In regards to the double output logging. It looks like minify is noticing the change to the source file, script.js, and minifying it to script.min.js (as expected). minify is then noticing the change to script.min.js and running again! minify is not ignoring its own output file!

My previously provided code could be modified as follows to ignore changes to the output file:

//...
if !verbose {
    Info.Println(file, "changed")
}

if !fileMatches(file) {
    break
}

//Ignore if file changed is the output file. Change was most likely caused by minify!
if file == output {
  break
}

task, err := NewTask(root, file, output, sync)
if err != nil {
    Error.Println(err)
    return 1
}
chanTasks <- task
//...

@tdewolff
Copy link
Owner

tdewolff commented Sep 17, 2022 via email

@c9845
Copy link
Author

c9845 commented Sep 20, 2022

Looked into this a bit further. It is not just JS (or TS) related, this also affects CSS. I imagine that means it effects all file types.

If I have a directory with styles.css on which I will use minify to create styles.min.css and I have another file, say test.txt, in that same directory, saving test.txt causes the "copy x to y" minify logging and the contents of styles.min.css are overwritten by the contents of test.txt. Saving styles.css causes the minified output to overwrite styles.min.css as you would expect.

@tdewolff
Copy link
Owner

Let me know if this fixes your issue. I've fixed various problems with watching file changes.

@c9845
Copy link
Author

c9845 commented Sep 22, 2022

Your recent changes do not resolve the issue. I continue to see the "copy 'x' to 'y'" logging output and the file being changed is copied to the minify output file.

@c9845
Copy link
Author

c9845 commented Sep 22, 2022

My mistake, your changes do fix the file copying. I had installed via go install github...@latest but it doesn't look like you tagged with a new version so the same, broken version was reinstalled! I recloned the source repo and go install-ed and this version does work properly (i.e.: no "copy 'x' to 'y') messages and minify output file is not overwritten with changed file's contents).

Thanks!

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

No branches or pull requests

2 participants