Skip to content

Commit f4d39fa

Browse files
authored
Merge pull request #4945 from sjrd/own-short-names-emitter-2
Fix #4482: Minify property names ourselves in fullLink when we don't use GCC.
2 parents c6dc8e1 + 280870d commit f4d39fa

File tree

33 files changed

+1195
-344
lines changed

33 files changed

+1195
-344
lines changed

Jenkinsfile

Lines changed: 65 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ def Tasks = [
185185
reversi$v/fastLinkJS \
186186
reversi$v/fullLinkJS \
187187
reversi$v/checksizes &&
188+
sbtretry ++$scala \
189+
'set Global/enableMinifyEverywhere := true' \
190+
reversi$v/checksizes &&
188191
sbtretry ++$scala javalibintf/compile:doc compiler$v/compile:doc library$v/compile:doc \
189192
testInterface$v/compile:doc testBridge$v/compile:doc &&
190193
sbtretry ++$scala headerCheck &&
@@ -199,68 +202,84 @@ def Tasks = [
199202
"test-suite-default-esversion": '''
200203
setJavaVersion $java
201204
npm install &&
202-
sbtretry ++$scala jUnitTestOutputsJVM$v/test jUnitTestOutputsJS$v/test testBridge$v/test \
205+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
206+
jUnitTestOutputsJVM$v/test jUnitTestOutputsJS$v/test testBridge$v/test \
203207
'set scalaJSStage in Global := FullOptStage' jUnitTestOutputsJS$v/test testBridge$v/test &&
204-
sbtretry ++$scala $testSuite$v/test $testSuite$v/testHtmlJSDom &&
205-
sbtretry 'set scalaJSStage in Global := FullOptStage' \
206-
++$scala $testSuite$v/test \
208+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
209+
$testSuite$v/test $testSuite$v/testHtmlJSDom &&
210+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
211+
'set scalaJSStage in Global := FullOptStage' \
212+
$testSuite$v/test \
207213
$testSuite$v/testHtmlJSDom &&
208-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withOptimizer(false))' \
209-
++$scala $testSuite$v/test &&
210-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withOptimizer(false))' \
214+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
215+
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withOptimizer(false))' \
216+
$testSuite$v/test &&
217+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
218+
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withOptimizer(false))' \
211219
'set scalaJSStage in Global := FullOptStage' \
212-
++$scala $testSuite$v/test &&
213-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= makeCompliant' \
214-
++$scala $testSuite$v/test &&
215-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= makeCompliant' \
220+
$testSuite$v/test &&
221+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
222+
'set scalaJSLinkerConfig in $testSuite.v$v ~= makeCompliant' \
223+
$testSuite$v/test &&
224+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
225+
'set scalaJSLinkerConfig in $testSuite.v$v ~= makeCompliant' \
216226
'set scalaJSStage in Global := FullOptStage' \
217-
++$scala $testSuite$v/test &&
218-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= makeCompliant' \
227+
$testSuite$v/test &&
228+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
229+
'set scalaJSLinkerConfig in $testSuite.v$v ~= makeCompliant' \
219230
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withOptimizer(false))' \
220-
++$scala $testSuite$v/test &&
221-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= { _.withSemantics(_.withStrictFloats(false)) }' \
222-
++$scala $testSuite$v/test &&
223-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= { _.withSemantics(_.withStrictFloats(false)) }' \
231+
$testSuite$v/test &&
232+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
233+
'set scalaJSLinkerConfig in $testSuite.v$v ~= { _.withSemantics(_.withStrictFloats(false)) }' \
234+
$testSuite$v/test &&
235+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
236+
'set scalaJSLinkerConfig in $testSuite.v$v ~= { _.withSemantics(_.withStrictFloats(false)) }' \
224237
'set scalaJSStage in Global := FullOptStage' \
225-
++$scala $testSuite$v/test &&
226-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= { _.withSemantics(_.withStrictFloats(false)) }' \
238+
$testSuite$v/test &&
239+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
240+
'set scalaJSLinkerConfig in $testSuite.v$v ~= { _.withSemantics(_.withStrictFloats(false)) }' \
227241
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withOptimizer(false))' \
228-
++$scala $testSuite$v/test &&
229-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withESFeatures(_.withAllowBigIntsForLongs(true)))' \
230-
++$scala $testSuite$v/test &&
231-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withESFeatures(_.withAllowBigIntsForLongs(true)).withOptimizer(false))' \
232-
++$scala $testSuite$v/test &&
233-
sbtretry \
242+
$testSuite$v/test &&
243+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
244+
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withESFeatures(_.withAllowBigIntsForLongs(true)))' \
245+
$testSuite$v/test &&
246+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
247+
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withESFeatures(_.withAllowBigIntsForLongs(true)).withOptimizer(false))' \
248+
$testSuite$v/test &&
249+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
234250
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withESFeatures(_.withAvoidLetsAndConsts(false).withAvoidClasses(false)))' \
235-
++$scala $testSuite$v/test &&
236-
sbtretry \
251+
$testSuite$v/test &&
252+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
237253
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withESFeatures(_.withAvoidLetsAndConsts(false).withAvoidClasses(false)))' \
238254
'set scalaJSStage in Global := FullOptStage' \
239-
++$scala $testSuite$v/test &&
240-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.CommonJSModule))' \
241-
++$scala $testSuite$v/test &&
242-
sbtretry \
255+
$testSuite$v/test &&
256+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
257+
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.CommonJSModule))' \
258+
$testSuite$v/test &&
259+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
243260
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.CommonJSModule))' \
244261
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallestModules))' \
245-
++$scala $testSuite$v/test &&
246-
sbtretry 'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.CommonJSModule))' \
262+
$testSuite$v/test &&
263+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
264+
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.CommonJSModule))' \
247265
'set scalaJSStage in Global := FullOptStage' \
248-
++$scala $testSuite$v/test &&
249-
sbtretry \
266+
$testSuite$v/test &&
267+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
250268
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \
251-
++$scala $testSuite$v/test &&
252-
sbtretry \
269+
$testSuite$v/test &&
270+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
253271
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallestModules))' \
254272
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \
255-
++$scala $testSuite$v/test &&
256-
sbtretry \
273+
$testSuite$v/test &&
274+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
257275
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleSplitStyle(ModuleSplitStyle.SmallModulesFor(List("org.scalajs.testsuite"))))' \
258276
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \
259-
++$scala $testSuite$v/test &&
260-
sbtretry \
277+
$testSuite$v/test &&
278+
# The following tests the same thing whether testMinify is true or false; we also set it for regularity.
279+
sbtretry ++$scala 'set Global/enableMinifyEverywhere := $testMinify' \
261280
'set scalaJSLinkerConfig in $testSuite.v$v ~= (_.withModuleKind(ModuleKind.ESModule))' \
262281
'set scalaJSStage in Global := FullOptStage' \
263-
++$scala $testSuite$v/test
282+
$testSuite$v/test
264283
''',
265284

266285
"test-suite-custom-esversion-force-polyfills": '''
@@ -504,9 +523,10 @@ mainScalaVersions.each { scalaVersion ->
504523
quickMatrix.add([task: "main", scala: scalaVersion, java: javaVersion])
505524
quickMatrix.add([task: "tools", scala: scalaVersion, java: javaVersion])
506525
}
507-
quickMatrix.add([task: "test-suite-default-esversion", scala: scalaVersion, java: mainJavaVersion, testSuite: "testSuite"])
526+
quickMatrix.add([task: "test-suite-default-esversion", scala: scalaVersion, java: mainJavaVersion, testMinify: "false", testSuite: "testSuite"])
527+
quickMatrix.add([task: "test-suite-default-esversion", scala: scalaVersion, java: mainJavaVersion, testMinify: "true", testSuite: "testSuite"])
508528
quickMatrix.add([task: "test-suite-custom-esversion", scala: scalaVersion, java: mainJavaVersion, esVersion: "ES5_1", testSuite: "testSuite"])
509-
quickMatrix.add([task: "test-suite-default-esversion", scala: scalaVersion, java: mainJavaVersion, testSuite: "scalaTestSuite"])
529+
quickMatrix.add([task: "test-suite-default-esversion", scala: scalaVersion, java: mainJavaVersion, testMinify: "false", testSuite: "scalaTestSuite"])
510530
quickMatrix.add([task: "test-suite-custom-esversion", scala: scalaVersion, java: mainJavaVersion, esVersion: "ES5_1", testSuite: "scalaTestSuite"])
511531
quickMatrix.add([task: "bootstrap", scala: scalaVersion, java: mainJavaVersion])
512532
quickMatrix.add([task: "partest-fastopt", scala: scalaVersion, java: mainJavaVersion])
@@ -527,7 +547,7 @@ otherScalaVersions.each { scalaVersion ->
527547
}
528548
mainScalaVersions.each { scalaVersion ->
529549
otherJavaVersions.each { javaVersion ->
530-
quickMatrix.add([task: "test-suite-default-esversion", scala: scalaVersion, java: javaVersion, testSuite: "testSuite"])
550+
quickMatrix.add([task: "test-suite-default-esversion", scala: scalaVersion, java: javaVersion, testMinify: "false", testSuite: "testSuite"])
531551
}
532552
fullMatrix.add([task: "partest-noopt", scala: scalaVersion, java: mainJavaVersion])
533553
fullMatrix.add([task: "partest-fullopt", scala: scalaVersion, java: mainJavaVersion])

linker-interface/shared/src/main/scala/org/scalajs/linker/interface/StandardConfig.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,19 @@ final class StandardConfig private (
4646
val relativizeSourceMapBase: Option[URI],
4747
/** Name patterns for output. */
4848
val outputPatterns: OutputPatterns,
49+
/** Apply Scala.js-specific minification of the produced .js files.
50+
*
51+
* When enabled, the linker more aggressively reduces the size of the
52+
* generated code, at the cost of readability and debuggability. It does
53+
* not perform size optimizations that would negatively impact run-time
54+
* performance.
55+
*
56+
* The focus is on optimizations that general-purpose JavaScript minifiers
57+
* cannot do on their own. For the best results, we expect the Scala.js
58+
* minifier to be used in conjunction with a general-purpose JavaScript
59+
* minifier.
60+
*/
61+
val minify: Boolean,
4962
/** Whether to use the Google Closure Compiler pass, if it is available.
5063
* On the JavaScript platform, this does not have any effect.
5164
*/
@@ -80,6 +93,7 @@ final class StandardConfig private (
8093
sourceMap = true,
8194
relativizeSourceMapBase = None,
8295
outputPatterns = OutputPatterns.Defaults,
96+
minify = false,
8397
closureCompilerIfAvailable = false,
8498
prettyPrint = false,
8599
batchMode = false,
@@ -148,6 +162,9 @@ final class StandardConfig private (
148162
def withOutputPatterns(f: OutputPatterns => OutputPatterns): StandardConfig =
149163
copy(outputPatterns = f(outputPatterns))
150164

165+
def withMinify(minify: Boolean): StandardConfig =
166+
copy(minify = minify)
167+
151168
def withClosureCompilerIfAvailable(closureCompilerIfAvailable: Boolean): StandardConfig =
152169
copy(closureCompilerIfAvailable = closureCompilerIfAvailable)
153170

@@ -173,6 +190,7 @@ final class StandardConfig private (
173190
| sourceMap = $sourceMap,
174191
| relativizeSourceMapBase = $relativizeSourceMapBase,
175192
| outputPatterns = $outputPatterns,
193+
| minify = $minify,
176194
| closureCompilerIfAvailable = $closureCompilerIfAvailable,
177195
| prettyPrint = $prettyPrint,
178196
| batchMode = $batchMode,
@@ -192,6 +210,7 @@ final class StandardConfig private (
192210
sourceMap: Boolean = sourceMap,
193211
outputPatterns: OutputPatterns = outputPatterns,
194212
relativizeSourceMapBase: Option[URI] = relativizeSourceMapBase,
213+
minify: Boolean = minify,
195214
closureCompilerIfAvailable: Boolean = closureCompilerIfAvailable,
196215
prettyPrint: Boolean = prettyPrint,
197216
batchMode: Boolean = batchMode,
@@ -209,6 +228,7 @@ final class StandardConfig private (
209228
sourceMap,
210229
relativizeSourceMapBase,
211230
outputPatterns,
231+
minify,
212232
closureCompilerIfAvailable,
213233
prettyPrint,
214234
batchMode,
@@ -237,6 +257,7 @@ object StandardConfig {
237257
.addField("relativizeSourceMapBase",
238258
config.relativizeSourceMapBase.map(_.toASCIIString()))
239259
.addField("outputPatterns", config.outputPatterns)
260+
.addField("minify", config.minify)
240261
.addField("closureCompilerIfAvailable",
241262
config.closureCompilerIfAvailable)
242263
.addField("prettyPrint", config.prettyPrint)
@@ -264,6 +285,7 @@ object StandardConfig {
264285
* - `sourceMap`: `true`
265286
* - `relativizeSourceMapBase`: `None`
266287
* - `outputPatterns`: [[OutputPatterns.Defaults]]
288+
* - `minify`: `false`
267289
* - `closureCompilerIfAvailable`: `false`
268290
* - `prettyPrint`: `false`
269291
* - `batchMode`: `false`

linker/jvm/src/main/scala/org/scalajs/linker/backend/closure/ClosureAstTransformer.scala

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,9 @@ private class ClosureAstTransformer(featureSet: FeatureSet,
210210
private def transformClassMember(member: Tree): Node = {
211211
implicit val pos = member.pos
212212

213-
def newFixedPropNode(token: Token, static: Boolean, name: Ident,
213+
def newFixedPropNode(token: Token, static: Boolean, name: MaybeDelayedIdent,
214214
function: Node): Node = {
215-
val node = Node.newString(token, name.name)
215+
val node = Node.newString(token, name.resolveName())
216216
node.addChildToBack(function)
217217
node.setStaticMember(static)
218218
node
@@ -258,7 +258,7 @@ private class ClosureAstTransformer(featureSet: FeatureSet,
258258
val node = newComputedPropNode(static, nameExpr, function)
259259
node.putBooleanProp(Node.COMPUTED_PROP_METHOD, true)
260260
node
261-
case name: Ident =>
261+
case name: MaybeDelayedIdent =>
262262
newFixedPropNode(Token.MEMBER_FUNCTION_DEF, static, name, function)
263263
}
264264

@@ -274,7 +274,7 @@ private class ClosureAstTransformer(featureSet: FeatureSet,
274274
val node = newComputedPropNode(static, nameExpr, function)
275275
node.putBooleanProp(Node.COMPUTED_PROP_GETTER, true)
276276
node
277-
case name: Ident =>
277+
case name: MaybeDelayedIdent =>
278278
newFixedPropNode(Token.GETTER_DEF, static, name, function)
279279
}
280280

@@ -290,7 +290,7 @@ private class ClosureAstTransformer(featureSet: FeatureSet,
290290
val node = newComputedPropNode(static, nameExpr, function)
291291
node.putBooleanProp(Node.COMPUTED_PROP_SETTER, true)
292292
node
293-
case name: Ident =>
293+
case name: MaybeDelayedIdent =>
294294
newFixedPropNode(Token.SETTER_DEF, static, name, function)
295295
}
296296

@@ -321,7 +321,7 @@ private class ClosureAstTransformer(featureSet: FeatureSet,
321321
args.foreach(arg => node.addChildToBack(transformExpr(arg)))
322322
node
323323
case DotSelect(qualifier, item) =>
324-
val node = Node.newString(Token.GETPROP, item.name)
324+
val node = Node.newString(Token.GETPROP, item.resolveName())
325325
node.addChildToBack(transformExpr(qualifier))
326326
setNodePosition(node, item.pos.orElse(pos))
327327
case BracketSelect(qualifier, item) =>
@@ -435,8 +435,8 @@ private class ClosureAstTransformer(featureSet: FeatureSet,
435435
val transformedValue = transformExpr(value)
436436

437437
val node = name match {
438-
case Ident(name, _) =>
439-
Node.newString(Token.STRING_KEY, name)
438+
case name: MaybeDelayedIdent =>
439+
Node.newString(Token.STRING_KEY, name.resolveName())
440440

441441
case StringLiteral(name) =>
442442
val node = Node.newString(Token.STRING_KEY, name)

linker/jvm/src/main/scala/org/scalajs/linker/backend/closure/ClosureLinkerBackend.scala

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,19 @@ final class ClosureLinkerBackend(config: LinkerBackendImpl.Config)
5454
s"Cannot use module kind $moduleKind with the Closure Compiler")
5555

5656
private[this] val emitter = {
57+
// Note that we do not transfer `minify` -- Closure will do its own thing anyway
5758
val emitterConfig = Emitter.Config(config.commonConfig.coreSpec)
5859
.withJSHeader(config.jsHeader)
5960
.withOptimizeBracketSelects(false)
6061
.withTrackAllGlobalRefs(true)
6162
.withInternalModulePattern(m => OutputPatternsImpl.moduleName(config.outputPatterns, m.id))
6263

63-
new Emitter(emitterConfig, ClosureLinkerBackend.PostTransformer)
64+
// Do not apply ClosureAstTransformer eagerly:
65+
// The ASTs used by closure are highly mutable, so re-using them is non-trivial.
66+
// Since closure is slow anyways, we haven't built the optimization.
67+
val postTransformer = Emitter.PostTransformer.Identity
68+
69+
new Emitter(emitterConfig, postTransformer)
6470
}
6571

6672
val symbolRequirements: SymbolRequirement = emitter.symbolRequirements
@@ -296,11 +302,4 @@ private object ClosureLinkerBackend {
296302
Function.prototype.apply;
297303
var NaN = 0.0/0.0, Infinity = 1.0/0.0, undefined = void 0;
298304
"""
299-
300-
private object PostTransformer extends Emitter.PostTransformer[js.Tree] {
301-
// Do not apply ClosureAstTransformer eagerly:
302-
// The ASTs used by closure are highly mutable, so re-using them is non-trivial.
303-
// Since closure is slow anyways, we haven't built the optimization.
304-
def transformStats(trees: List[js.Tree], indent: Int): List[js.Tree] = trees
305-
}
306305
}

0 commit comments

Comments
 (0)