Skip to content

Commit

Permalink
Fix scala-js#40: Generate valid path names for require-$name.js files.
Browse files Browse the repository at this point in the history
We used to embed the target URI in the file name, which generates
invalid paths. This used to be fine when we used our own virtual
files, whose in-memory files accepted any string as path. Since we
are now using Jimfs, we need to use correct path names.

We now generate file names based on the `commonJSName`. Since
`commonJSName` must be a valid JavaScript identifier, it is also a
valid file name (in particular, it cannot contain the characters
`: ; / \`). In the unlikely event that two libraries have the same
`commonJSName` (which would probably be erroneous anyway, but
still), we use suffixes to generated distinct file names.
  • Loading branch information
sjrd committed May 22, 2020
1 parent 1342837 commit 322beec
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ object JSDependenciesPlugin extends AutoPlugin {

}

private lazy val memFS = Jimfs.newFileSystem()

import autoImport._

/** Collect certain file types from a classpath.
Expand Down Expand Up @@ -131,6 +129,7 @@ object JSDependenciesPlugin extends AutoPlugin {
import java.util.zip._

val jarPath = jar.getPath
val memFS = Jimfs.newFileSystem()

val stream =
new ZipInputStream(new BufferedInputStream(new FileInputStream(jar)))
Expand Down Expand Up @@ -401,12 +400,25 @@ object JSDependenciesPlugin extends AutoPlugin {
*/
val libs = env match {
case _: org.scalajs.jsenv.nodejs.NodeJSEnv =>
val memFS = Jimfs.newFileSystem()
val usedNames = mutable.Set.empty[String]

def getFileNameFor(commonJSName: String): String = {
var name = commonJSName
var suffix = 0
while (!usedNames.add(name)) {
suffix += 1
name = s"${commonJSName}_$suffix"
}
s"require-$name.js"
}

for (dep <- deps) yield {
dep.info.commonJSName.fold {
dep.lib
} { commonJSName =>
val fname = materialize(dep.lib).toASCIIString
Files.write(memFS.getPath(s"require-$fname"),
Files.write(memFS.getPath(getFileNameFor(commonJSName)),
s"""$commonJSName = require("${escapeJS(fname)}");""".getBytes(StandardCharsets.UTF_8))
}
}
Expand Down
1 change: 1 addition & 0 deletions sbt-plugin-test/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ name := "sbt-plugin-test"
addCommandAlias("testAll",
";jsDependenciesTest/packageJSDependencies" +
";jsDependenciesTest/packageMinifiedJSDependencies" +
";jsDependenciesTest/jsEnvInput" + // #40
";jsDependenciesTest/regressionTestForIssue2243" +
";jsNoDependenciesTest/regressionTestForIssue2243")

Expand Down

0 comments on commit 322beec

Please sign in to comment.