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

javascript with calls to String.raw gets incorrectly minified #701

Closed
perrin4869 opened this issue May 15, 2024 · 17 comments
Closed

javascript with calls to String.raw gets incorrectly minified #701

perrin4869 opened this issue May 15, 2024 · 17 comments

Comments

@perrin4869
Copy link
Contributor

I put together a small reproduction here

@perrin4869 perrin4869 changed the title javascript with calls to String.raw gets incorretly minified javascript with calls to String.raw gets incorrectly minified May 15, 2024
@tdewolff
Copy link
Owner

Thanks Julian, should be fixed now!

@perrin4869
Copy link
Contributor Author

thanks for the lightning speed fix 😄
when you have time, can you update the npm module? :)

@tdewolff
Copy link
Owner

Just churned out a new release, the NPM release should come up shortly after the GitHub action finishes.

@perrin4869
Copy link
Contributor Author

seems the CI failed because the macos image doesn't have golang installed?

@tdewolff
Copy link
Owner

This is now fixed in the latest release!

@perrin4869
Copy link
Contributor Author

perrin4869 commented May 17, 2024

Yeah, I was trying it earlier, thank so much!!
Only thing is, I am running into a problem where the fix doesn't seem to be applied in my CI, and I think it is because it is running arm Linux, the fix is working perfectly on my x86 Linux desktop... any clues?
Edit: this is related to the node binding

@tdewolff
Copy link
Owner

Are you sure it is downloading the latest commit from GitHub? Maybe add a go get -u github.com/tdewolff/minify/v2@v2.20.24 or @latest to make sure it uses the latest version (I've noticed it does some caching sometimes).

Otherwise, I believe that x86 will use the precompiled binaries from the GitHub workflow, but ARM will probably need to build from source, but that should just work...not sure what the problem could be...

@perrin4869
Copy link
Contributor Author

maybe it is because this is not updated?

github.com/tdewolff/minify/v2 v2.12.5

Since there are no prebuilds for arm on linux, it would be building the bindings from here, right?

@perrin4869
Copy link
Contributor Author

btw, it seems we can soon run arm linux github actions :)
https://github.blog/changelog/2023-10-30-accelerate-your-ci-cd-with-arm-based-hosted-runners-in-github-actions/

@tdewolff
Copy link
Owner

You're right, I've made some changes, do they look good to you? If so, I'll release a
new version for you to test.

PS: ARM support would be a great relieve!

@perrin4869
Copy link
Contributor Author

I don't think those would work, because the makefile is calling git to produce the hash, but the node module isn't a git repository... I think you'd need to get those values and inject them into the make file at build time

@tdewolff
Copy link
Owner

Or perhaps just go get -u github.com/tdewolff/minify/v2@latest, that should get the latest version...the whole reason of the workflows and build scripts was to avoid changing the contents for every release (ie. I release a new version and the next commit involves updating the new version throughout all build files).

@perrin4869
Copy link
Contributor Author

right, this is not a fun problem to solve ^^;;
the only problem with go get -u github.com/tdewolff/minify/v2@latest is that you would get unpredictable versions of the minifier...
I think what could work in a predictable fashion, is getting the version from the package.json file, perhaps via node or grep? But just using latest wouldn't be bad either, just not ideal

@perrin4869
Copy link
Contributor Author

node -e 'console.log(require("./package.json").version)' seems to do the trick

@tdewolff
Copy link
Owner

I've pushed out a commit, it will work with the next version (or do you need me to release a new version?).

@perrin4869
Copy link
Contributor Author

Thanks man! That's very helpful!!
Yeah, if you publish a new version I can update my code, otherwise I'll wait 🙂

@tdewolff
Copy link
Owner

I've just released v2.20.25 for you, let me know if it works!

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