scalameta / mdoc

Typechecked markdown documentation for Scala
https://scalameta.org/mdoc/
Apache License 2.0
394 stars 80 forks source link

WIP: Remap es imports - help needed... #882

Closed Quafadas closed 1 month ago

Quafadas commented 1 month ago

This is a currently messy branch which aims to remap ESmodule imports at link time.

It feels quite close to doing what I want (not near ready for merging, lots of mess atm). Unfortunately, I'm stuck. It compiles, but doesn't work, sadly. The extra test I wanted for the CLI, fails with this message

tests.js.JsCliSuite:
==> X tests.js.JsCliSuite.remap  1.62s java.lang.NoClassDefFoundError: com/armanbilge/sjsimportmap/ImportMappedIRFile$
Caused by: java.lang.ClassNotFoundException: com.armanbilge.sjsimportmap.ImportMappedIRFile$

If I publish local, then I can't get any information (logs, printings etc are ignored) out of the ScalaJSWorker - I suspect it may be failing with a similar problem but not telling me.

I've added the key dependancy "com.armanbilge" %%% "scalajs-importmap" % "0.1.1" cross CrossVersion.for3Use2_13 to the build definition, so I'm afraid I'm stumped on why it compiles, but doesn't seem to then have the right classpath.

@tgodzik Would you happen to have a clue / hint what could be happening ?

Quafadas commented 1 month ago

I suspect, that I may need to armans remap dep here;

https://github.com/Quafadas/mdoc/blob/2fae61dcf4b1ef6d6e790347b69fde044bbf8889/mdoc-js/src/main/scala/mdoc/modifiers/JsModifier.scala#L92

But I have no idea, how...

tgodzik commented 1 month ago

To add the deps you would need to add them here https://github.com/scalameta/mdoc/blob/main/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala#L165

Though ideally can we inline the code instead of depending on a separate libary?

Quafadas commented 1 month ago

To add the deps you would need to add them here https://github.com/scalameta/mdoc/blob/main/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala#L165

Oooooh - Thankyou! I haven't yet tested it but it looks like that's the hint I needed. Hopefully I can look tomorrow.

Though ideally can we inline the code instead of depending on a separate libary?

The obvious downside of inlining the code, is that I would doubt I'm capable of maintaining or probably, understanding it... so I'd personally take advantage of the library approach. However...

... you and Arman are on a different skill level to myself :-), and if your view is inlining makes maintenance easier I'll try and do it ... it wasn't much code when I looked, but it's obviously then in two places.

tgodzik commented 1 month ago

Maybe inlining is not necessary, it just felt that we didn't have any external deps before, so this could be problematic, but on the other maybe not. We can come back to it later

Quafadas commented 1 month ago

To add the deps you would need to add them here https://github.com/scalameta/mdoc/blob/main/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala#L165

Though ideally can we inline the code instead of depending on a separate libary?

Hmmm - is this for the SBT plugin? I was hoping to be able to do this out of the CLI... my understanding is that the SBT plugin is then built on top of that?

tgodzik commented 1 month ago

To add the deps you would need to add them here https://github.com/scalameta/mdoc/blob/main/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala#L165 Though ideally can we inline the code instead of depending on a separate libary?

Hmmm - is this for the SBT plugin? I was hoping to be able to do this out of the CLI... my understanding is that the SBT plugin is then built on top of that?

I am pretty sure this would need to be added via option --classpath then, but I haven't experimented with that

tgodzik commented 1 month ago

Wait, isn't it explained here actually https://scalameta.org/mdoc/docs/js.html#command-line <- looks like you need a properties file populated beforehand

Quafadas commented 1 month ago

Yes, this was my original angle of attack;

js-classpath=/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-library_2.13/1.16.0/scalajs-library_2.13-1.16.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-dom_sjs1_3/2.8.0/scalajs-dom_sjs1_3-2.8.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/raquo/laminar-shoelace_sjs1_3/0.1.0/laminar-shoelace_sjs1_3-0.1.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/raquo/laminar_sjs1_3/17.0.0/laminar_sjs1_3-17.0.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.13/scala-library-2.13.13.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-javalib/1.16.0/scalajs-javalib-1.16.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-scalalib_2.13/2.13.13%2B1.16.0/scalajs-scalalib_2.13-2.13.13%2B1.16.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_sjs1_3/3.3.3/scala3-library_sjs1_3-3.3.3.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/raquo/airstream_sjs1_3/17.0.0/airstream_sjs1_3-17.0.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/raquo/ew_sjs1_3/0.2.0/ew_sjs1_3-0.2.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/app/tulz/tuplez-full-light_sjs1_3/0.4.0/tuplez-full-light_sjs1_3-0.4.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/armanbilge/scalajs-importmap_2.13/0.1.1/scalajs-importmap_2.13-0.1.1.jar
js-scalac-options=-scalajs
js-linker-classpath=/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scalameta/mdoc-js-worker_3/2.5.4/mdoc-js-worker_3-2.5.4.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-linker_2.13/1.16.0/scalajs-linker_2.13-1.16.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/armanbilge/scalajs-importmap_2.13/0.1.1/scalajs-importmap_2.13-0.1.1.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scalameta/mdoc-js-interfaces/2.5.4/mdoc-js-interfaces-2.5.4.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_3/3.3.3/scala3-library_3-3.3.3.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.13/scala-library-2.13.13.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-linker-interface_2.13/1.16.0/scalajs-linker-interface_2.13-1.16.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-ir_2.13/1.16.0/scalajs-ir_2.13-1.16.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/com/google/javascript/closure-compiler/v20220202/closure-compiler-v20220202.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-parallel-collections_2.13/0.2.0/scala-parallel-collections_2.13-0.2.0.jar:/Users/simon/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-logging_2.13/1.1.1/scalajs-logging_2.13-1.1.1.jar
js-module-kind=ESModule

The problem seems to be twofold - I can't get any logging information out of ScalaJsWorker. println, logger.log etc all seems to be re-directed elsewhere, so although this actually compiles and runs, it doesn't do what I want, and I can't figure out why!

Hence, trying to get it working in context which is introspectable in the IDE... somehow!

Quafadas commented 1 month ago

So I've figured out, that something is creating an mdoc.properties file as a resource that the tests are consuming. And that's how it all hangs together. As that file doesn't include the remap dep, I'm now increasingly sure that why I'm struggling. Can't figure out what is creating that file though, so some work still to do.

Quafadas commented 1 month ago

Okay, I think I finally understand. Your original note on the MdocPlugin, was (predicatably) correct.

I would say that I find the dependance confusing however - the sbt plugin itself depends on the CLI. So I had assumed the CLI was upstream of it.

However, the CLI tests depend on correct configuration of the sbt plugin. The SBT plugin generates an mdoc.properties file during as a part of it's resourceGenerators build step. The CLI tests assume they are going to find that mdoc.properties file on the classpath.

This may not be how I would have chosen to structure things - it was not intuitive for me. Having understood that though, I think I can proceed. Apologies for the noise.

Quafadas commented 1 month ago

Although I think this does have a compatibility implication actually - Although the API surface wouldn't change and this could appear binary compatible, someone using the CLI on "new mdoc" published with this functionality would almost certainly get unintelligible errors without updating their mdoc.properties file.

An SBT user would be fine to upgrade though. Awkward - to be discussed later. Ah - and now we're back to your "inline the code" suggestion.

Quafadas commented 1 month ago

I've inlined that code, and I'm stuck with this on a publish local;

info: Compiling 2 files to /Users/simon/Code/mdoc_test/out
Exception in thread "main" java.lang.NoSuchMethodError: 'byte[] org.scalajs.ir.Names$.org$scalajs$ir$Names$$validateSimpleEncodedName(byte[])'
        at org.scalajs.ir.Names$SimpleFieldName$.apply(Names.scala:168)
        at org.scalajs.ir.Names$SimpleFieldName$.apply(Names.scala:171)
        at org.scalajs.linker.backend.emitter.EmitterNames$.<clinit>(EmitterNames.scala:29)
        at org.scalajs.linker.backend.emitter.Emitter$.$anonfun$symbolRequirements$1(Emitter.scala:1257)

I think I'm going to have to down tools on this - my biggest problem is the way that mdoc configures the both CLI and its settings file, which it assumes has a fixed name and is on the class path. This combination is hard to work with. Time to take a step back.

tgodzik commented 1 month ago

Looks like maybe a Scala JS version mismatch?

Quafadas commented 1 month ago

This was the command used to generate the class paths;

cat <<EOT > mdoc.properties
js-classpath=$(coursier fetch org.scala-js:scalajs-library_2.13:1.16.0 org.scala-js:scalajs-dom_sjs1_3:2.8.0 com.raquo:laminar-shoelace_sjs1_3:0.1.0 com.raquo:laminar_sjs1_3:17.0.0 -p)
js-scalac-options=-scalajs
js-linker-classpath=$(coursier fetch org.scalameta:mdoc-js-worker_3:0.0.0+1377-32535440+20240711-1446-SNAPSHOT org.scala-js:scalajs-linker_2.13:1.16.0 -p)
js-module-kind=ESModule
EOT
Quafadas commented 1 month ago

I cannot for the life of me figure out, how "mdoc.properties" is getting on the class path. I can see where it's generated, and I've generated a second properties file in exactly the same place, but apparently, that one isn't on the class path.