kilork / actix-web-static-files

actix-web static files as resources support
The Unlicense
75 stars 18 forks source link

Allow for NpmBuild install output to be logged #33

Closed Chaostheorie closed 3 years ago

Chaostheorie commented 3 years ago

At the moment all output (to stdout) is discarded and unreachable for a developer. The appropiate function, from my POV, would be NpmBuild::install.

I would love to either specify some file to write the log or to have it output to stdout.

Outputting to stdout or some kid of logger would allow the output to be shown in the cargo debug logs. By defualt there's no need to do any further catching as cargo will prevent any logs to be shown by build scripts.See this issue for more information on this topic.

Chaostheorie commented 3 years ago

For at least npm commands run from bash it's also possible to use "command": "bash script.sh >> compile.log" to redirect stdout to a file. Though I'm not to sure how this would behave on windows and mac.

I would strongly suggest though to write to the cargo log instead.

kilork commented 3 years ago

Hi, yes, I changed this at the latest release, if it is really blocking you - just switch to previous version.

Why I changed this: seems like (but I am not 100% sure, I would probably double check) this prevents change detection instructions to work. Because latest release adds exactly this feature, and this is really nice feature - allows really make builds and IDE faster, I decided to discard log output.

But I think you suggestion about logging to file is nice, I will experiment on this.

Just for curiosity, do you find NpmBuild functionality important, or maybe we should just drop this? I received recently request to drop it completely, want to know some feedback from users.

Chaostheorie commented 3 years ago

Thank you for the fast reply. My build is luckily not blocked because I can redirect all output (from stdout) to the file with above mentioned redirect.

I personally find NpmBuild functionality pretty important for new users since using npm, or webpack for these matters, is very popular and most likely the reason someone would want to embed their static assets in their binary. Though I can also understand why it may seem unnecessary for some users that don't rely on npm or another package manager. I would guess making it a default feature instead would be a possible option to simply give developers to choice.

Though said that I really appreciate you caring about IDE performance. I didn't know that the output could affect IDE's though it certainly makes sense to drop the output then. Maybe some note on the readme that output from e.g. webpack will not be shown would be nice though (If it isn't already there and I didn't see the note).

kilork commented 3 years ago

IDE performance is actually a thing, which surprised me also. But it really happens. It is a side effect, but I was struggling with this for a while, and last version was just a very first step in this direction. It does not work automatically at the moment to not break backward compatibility. I plan to release version 3.1 with some breaking changes in default options.

We will find a way, to improve the user experience, for me it is most important feedback and I think it is really valuable.

Chaostheorie commented 3 years ago

Happy to hear your plans for future releases. I personally support breaking changes, if they solve a problem, and I'm eager to test the new release when it's published. Thank you for your work on Open Source Libraries. It's really appreciated!

stale[bot] commented 3 years ago

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.

kilork commented 3 years ago

I think we fix this soon.

stale[bot] commented 3 years ago

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.

Chaostheorie commented 3 years ago

Will close this for now since a mitigation has been found (piping output of commands to files)