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

Improve GCC/ Minifier documentation #645

Open
OndrejSpanel opened this issue Mar 21, 2024 · 10 comments
Open

Improve GCC/ Minifier documentation #645

OndrejSpanel opened this issue Mar 21, 2024 · 10 comments

Comments

@OndrejSpanel
Copy link
Contributor

OndrejSpanel commented Mar 21, 2024

I would like to experiment with the new minifier a bit. How can I control if the new minifier or GCC is used?

I find the documentation about scalaJSLinkerConfig a bit lacking, esp. when Google often shows me pages which are quite old.

The documentation I am aware of:

https://www.scala-js.org/doc/project/module.html
https://www.scala-js.org/doc/internals/compile-opt-pipeline.html

scala-js/scala-js#4482 mentions:

We automatically enable the new minifier under fullLink when GCC is disabled. This can be overridden with a scalaJSLinkerConfig setting.

There is no more explanation.

In LinkerBackendImpl.scala I can see some functions which look they might do something:

withClosureCompilerIfAvailable
withMinify

Similar properties can be seen in https://www.scala-js.org/api/scalajs-linker-interface-js/latest/org/scalajs/linker/interface/StandardConfig.html, but there is no documentation for them.

I guess when I want to use the new minifier I want:

fullOptJS / scalaJSLinkerConfig ~= (_.withClosureCompiler(false).withMinify(true))
@gzm0
Copy link
Contributor

gzm0 commented Mar 21, 2024

Yes. Also, the config is documented in source:
linker-interface/shared/src/main/scala/org/scalajs/linker/interface/StandardConfig.scala

Seems like the scaladoc comments are not in the right place. We should obviously fix that.

@OndrejSpanel
Copy link
Contributor Author

I do not think the GCC stops being used unless I use scalaJSLinkerConfig ~= { _.withModuleKind(ModuleKind.ESModule) }. With CommonJSModule I always see [info] Closure: 0 error(s), 0 warning(s) and the filesize is exactly what is was before.

@gzm0
Copy link
Contributor

gzm0 commented Mar 21, 2024

Ah, right. I think you need to scope it in the config as well:

Compile / fullLinkJS / scalajsLinkerConfig

@OndrejSpanel
Copy link
Contributor Author

OndrejSpanel commented Mar 21, 2024

Confirmed, with Compile / fullLinkJS I can choose the minifier.

Why is the Compile scope not needed for withModuleKind, but is needed for withMinify? How about withOptimizer - does it need the Compile scope as well?

I am checking some other basic documentation and tutorials. If you clone https://github.com/scala-js/scalajs-tutorial, it uses ancient versions of sbt and Scala.js, IntelliJ is unable to import the project. I can prepare PR for that if desired.

A bit related: it seems the minifier assumes some other JS minification will be done later in the pipeline, at least its ScalaDoc says so. Would this also be in scope of the Basic Tutorial?

@gzm0
Copy link
Contributor

gzm0 commented Mar 23, 2024

Why is the Compile scope not needed for withModuleKind, but is needed for withMinify?

Because of how we configure the fullLink config:

https://github.com/scala-js/scala-js/blob/80eafa9ecb54f6763c0df32350c9c6cd8e7343c5/sbt-plugin/src/main/scala/org/scalajs/sbtplugin/ScalaJSPluginInternal.scala#L468-L475

You'll notice this is scoped in both the configuration and the task. I guess at some point in the past, we decided (explicitly or implicitly) it's more important to be able to re-configure configs easily then re-configure fast v.s. full defaults easily. (I'm making the assumption here that sbt axis are not hierarchical but I believe this is the case).

I can prepare PR for that if desired.

Always appreciated. Please do note that the commit structure of that repo is a bit odd: The commits correspond to steps. So you essentially need to rewrite its entire history to update it.

Would this also be in scope of the Basic Tutorial?

IMO no. The basic tutorial is officially "old" (as you can see from the Tutorial Overview). The Scala.js and Vite tutorial does perform JS minification (or more precisely Vite's production build does).

@OndrejSpanel
Copy link
Contributor Author

Thanks for the explanation.

How about withOptimizer - does it need the Compile scope as well?

Still unclear about withOptimizer(false) - should it be used from Compile / fastOptJS or just fastOptJS?

@sjrd
Copy link
Member

sjrd commented Mar 24, 2024

First, always use fastLinkJS/fullLinkJS when configuring, even if you use the Opt tasks in the end.

Second, just fastLinkJS / never does anything. It's always Compile / fastLinkJS /.

@OndrejSpanel
Copy link
Contributor Author

Explanation is good, but the main point of this issue is to improve documentation so that the explanation here is not necessary.

Besides the links I have provided, some related documentation seems to be present in https://www.scala-js.org/doc/project/building.html as well. However I doubt I want to have this in my SBT:

scalaJSLinkerConfig ~= { _.withOptimizer(false) }

I would almost certanly want to disable optimizer for fastOpt only, because that is where I expect to do my debugging.

@OndrejSpanel
Copy link
Contributor Author

First, always use fastLinkJS/fullLinkJS when configuring

I have tried it now with the Scala.js Tutorial project.

This affects fastOptJS output:

Compile / fastOptJS / scalaJSLinkerConfig ~= (_.withOptimizer(false))
Compile / fastLinkJS / scalaJSLinkerConfig ~= (_.withOptimizer(false))
fastOptJS / scalaJSLinkerConfig ~= (_.withOptimizer(false))

This does not:

fastLinkJS / scalaJSLinkerConfig ~= (_.withOptimizer(false))

@gzm0
Copy link
Contributor

gzm0 commented Mar 31, 2024

Regarding the doc of StandardConfig: I've attempted to move the doc comments to @params to make them appear in Scaladoc (based off scala/bug#577), but it seems this has no effect :-/ I do not know where the doc comments for the fields should go.

@sjrd sjrd transferred this issue from scala-js/scala-js Aug 29, 2024
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

3 participants