- 
                Notifications
    
You must be signed in to change notification settings  - Fork 32
 
Description
I set up a sample repository to show step by step what I consider a problem.
git clone [email protected]:mkurz/same-webjar-different-type.git- Read the step by step guide I set up:
 
(Copy pasting the READMEs here in case I ever delete that reproducer repo, click to expand)
We include three webjars, they have the same name, but different groupid and **different versions**:
 libraryDependencies ++= Seq( 
   "org.webjars.npm" % "prelude-ls" % "1.2.1", 
   "org.webjars.bower" % "prelude-ls" % "1.1.2", 
   "org.webjars" % "prelude-ls" % "1.1.1", 
 ) If you run:
$ sbt "clean; assets"
$ tree -L 1 ./target/web/web-modules/main/webjars/lib/prelude-ls
./target/web/web-modules/main/webjars/lib/prelude-ls
├── 1.1.1
├── 1.1.2
└── 1.2.1
4 directories, 0 filesYou can see for each version a subfolder gets created.
This wasn't the case before webjars-locator-core#28 was merged.
Before that pull request, no subfolders got created, but all three webjars were merged directly into the prelude-ls folder.
So IMHO creating subfolders is the better approach now.
If you comment out two of the three webjars, so that only one webjars remains, then also no subfolders will be created, but the content of the one webjar will be put directly into the prelude-ls folder. This behaviour also seems reasonable, given the history of how it was done before.
But what if the exact same version is used for the above three webjars?
Lets find out in the same-webjar_same-version branch, take a look at its README!
In this branch we include three webjars, they have the same name and the same versions (but different groupid):
libraryDependencies ++= Seq(
  "org.webjars.npm" % "prelude-ls" % "1.1.1",
  "org.webjars.bower" % "prelude-ls" % "1.1.1",
  "org.webjars" % "prelude-ls" % "1.1.1",
)Again, run:
$ git checkout same-webjar_same-version
$ sbt "clean; assets"
$ tree -L 1 ./target/web/web-modules/main/webjars/lib/prelude-ls
./target/web/web-modules/main/webjars/lib/prelude-ls
├── browser                   <-- folder from bower webjar
├── CHANGELOG.md              <-- file from bower webjar
├── .gitignore                <-- file from bower webjar
├── lib                       <-- folder from npm or bower webjar (who knows?)
├── LICENSE                   <-- file from npm or bower webjar (who knows?)
├── Makefile                  <-- file from bower webjar
├── package.json              <-- file from npm or bower webjar (who knows?)
├── package.json.ls           <-- file from bower webjar
├── prelude-browser.js        <-- file from classic webjar
├── prelude-browser-min.js    <-- file from classic webjar
├── preroll.ls                <-- file from bower webjar
├── README.md                 <-- file from npm or bower webjar (who knows?)
├── src                       <-- folder from bower webjar
├── test                      <-- folder from bower webjar
└── .travis.yml               <-- file from bower webjar
5 directories, 11 filesNow no subfolders get created (of course, it would be the same subfolder because they all have the same version).
But now the files of all three webjars are merged directly into the prelude-ls folder (see notes above).
IMHO this is not a good idea, because who knows which webjar overrides which? Which file(s) get(s) overriden?
I think such a conflict should fail hard with an exception like
Version conflict: WebJar "prelude-ls" with version "1.1.1" is present multiple times but with different groupids.
The thing is if I write an application or a library and include for example
"org.webjars" % "prelude-ls" % "1.1.1"`I expect that this dependency (or better: the files from this WebJar) is immutable if it is present. Now maybe the files don't change for a while, but  maybe I add another dependency to my project which pulls in prelude-ls 1.1.1, but the npm one... Now maybe that would now override the files and suddenly things can get weird...
This is why I think such a case should be disallowed.
@jamesward What do you think? Shouldn't the described case raise an exception since it's a version conflict? Specially that the contents of the files is not really immutable I think this is very very bad behaviour...
Side note on creating subfolders
Like mentioned above, subfolders were not always created in the past, but a relatively new behaviour, at least from Play's perspective, because in Play 2.8 we use an older webjars-locator version which did not create subfolders (like demonstrated, WebJar's contents got merged into one folder), whereas Play 2.9+ now uses a newer webjars-locator version which now creates subfolders.
What I experienced now is that using require(foolib) failed, because in Play 2.8, even when there was a "conflict", foolib would be directly available, even if the folder contents would be unpredictable, because WebJars could have been merged together. But now in Play 2.9+, because a project did actually pull in multiple webjars with the same name, subfolders for each version get created, so require(foolib) suddenly failed. To fix that I had to write require(foolib/1.2.3) (1.2.3 being the version). However... how would I know, if I write an sbt web plugin, if there is another sbt web library pulled in by a project which pulls in the same webjar, causing subfolders to be created? Like I can never be sure if I have to write require(foolib/1.2.3) or require(foolib), I never know for sure where the WebJar gets extracted too...
To solve this we need to come up with a fallback solution, which first looks for the WebJar in the subfolder and then, if that sub folder does not exist, in the parent folder. This way it is guaranteed require(...) doesn't fail - and actually also, if you do the lookup in that order, that you definitely are accessing the exact version which got defined in the sbt libraryDependencies (because if the version subfolder does not exists, it means there is only one webjar with that name pulled in and extracted in the parent folder, the one you defined in your project).
I implemented such a solution with the help of the node-require-fallback package (which comes with a very simple implemention), see here:
- https://github.com/sbt/sbt-coffeescript/blob/2886577f0d0dce23b42d80f2f371bcba077058a7/src/main/resources/coffee.js#L10-L11
 - Full pull request including a scripted test to make that solution bulletproof: Handle multiple extracted versions of same webjar sbt/sbt-coffeescript#33
 
@jamesward What do you think about all of that?