-
Notifications
You must be signed in to change notification settings - Fork 5
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
jsEnvInput crashes when one of the JS libs has a commonJSName #40
Labels
Comments
sjrd
added a commit
to sjrd/jsdependencies
that referenced
this issue
May 21, 2020
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.
sjrd
added a commit
to sjrd/jsdependencies
that referenced
this issue
May 22, 2020
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.
sjrd
added a commit
that referenced
this issue
May 22, 2020
Fix #40: Generate valid path names for `require-$name.js` files.
@japgolly This is now fixed in the released version 1.0.1. It's on its way to Maven Central. |
Sweet! Thanks @sjrd ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reproducible with the following command in the
sbt-plugin-test
directory:causes the following error:
That's on Windows. On Linux a different error is thrown, but for essentially the same reasons:
In both cases, the issue is that the following line:
jsdependencies/jsdependencies-sbt-plugin/src/main/scala/org/scalajs/jsdependencies/sbtplugin/JSDependenciesPlugin.scala
Lines 409 to 410 in 1342837
attempts to create a
Path
with an illegal file name.This used to be fine when we had our own virtual file abstraction, whose in-memory files accepted any string as path. It's not OK anymore, now that we use Jimfs.
The text was updated successfully, but these errors were encountered: