badoo / Reaktive

Kotlin multi-platform implementation of Reactive Extensions
Apache License 2.0
1.18k stars 58 forks source link

Publish Reaktive node.js target to npm #488

Closed minaEweida closed 3 years ago

minaEweida commented 4 years ago

Is there any plans to publish Reaktive to npm? Recently we used reaktive in our lib to support node.js apps so we are publishing Reaktive to our internal artifactory since Reaktive is not published

mpetuska commented 3 years ago

You can't avoid that unfortunatelly. However the only downside of this behaviour is bloated npm mundle size. Other than that the duplicate versions of these objects accross your modules will not conflict if they appear on the same consumer project as they're isoladed under their own separate module "namespaces"

gciatto commented 3 years ago

@arkivanov yes, I think you should prevent that. I mean: if you partitioned your code into inter-dependent modules in Kotlin, I believe the same partitioning should be translated into the Node projects.

I'm struggling as well to understand the functioning of the binaries.library() and binaries.executable() commands. So far, I noticed a behaviour that is different than the @mpetuska was describing above.

Maybe I'm wrong, yet I'll do more tests and report them here

gciatto commented 3 years ago

However the only downside of this behaviour is bloated npm mundle size.

@mpetuska, I disagree: if they've designed the utils library to expose some classes and functions, the Node-transpiled version of the utils library should expose the same classes and functions, IMHO. Otherwise, the benefits of partitioning the code into different packages are lost.

mpetuska commented 3 years ago

@gciatto-unibo I totally agree. All I'm saying is that I don't think it's possible to achieve with current IR backend.

gciatto commented 3 years ago

@mpetuska oh that would be a shame.

Just to be clear, I totally trust your experience, @mpetuska, but I simply can't see why JetBrains should discourage the creation of Node libraries... especially considering that it was possible with the legacy backend.

As I said, I'll perform some experiments. I definitely care about understanding how the IR backend works. Now this is personal :smiley:

arkivanov commented 3 years ago

Maybe I'm wrong, yet I'll do more tests and report them here

@gciatto-unibo I will appreciate an update from you, thanks.

I don't think it's possible to achieve with current IR backend.

I suppose the Legacy mode will go away eventually, and we should look towards IR mode? Maybe someome, as a publishing plugins developer, could file an issue to YT? This sounds like a valid concern, perhaps they could improve it. Otherwise I could do it myself once everything is clear, but I may have problems explaining everything clear.

mpetuska commented 3 years ago

@arkivanov if you toggle both mode via gradle.properties by setting kotlin.js.compiler=both, you could easily overwrite it only for npm publishing in your CI to IR mode by setting ./gradlew myNpmPublishTask -Pkotlin.js.compiler=ir. That way you still publish maven dependencies in BOTH mode, but use IR mode for npm publishing to reap all its benefits (since BOTH mode doesn't make any sense for npm library)

arkivanov commented 3 years ago

@mpetuska Yeah, this should work. Also we can pass a custom flag and set the mode in the code. The biggest issue we have so far is the code from dependency modules being bundled.

mpetuska commented 3 years ago

Yeah, I know :/ I've just forked your repo to give it a go. Hopefully I'll be able to raise a sample PR by the weekend.

arkivanov commented 3 years ago

@mpetuska we appreciate your efforts and looking forward for the PR!

mpetuska commented 3 years ago

@arkivanov There you go - a PR for your inspection

mpetuska commented 3 years ago

You can't go around kotlin dependencies bundling, unfortunately, but you still get fully functional npm modules. And the size is not that big of an issue for node.js, only browser consumers would feel the bloat.

arkivanov commented 3 years ago

@mpetuska Thanks for the PR! I'm on vacation till the end of this week, so I I will likely check it next week.

Do you think it makes sense to publish with bundled dependencies? Will there be any issues when using different modules at the same time? E.g. reaktive with bundled utils and utils module itself? Will bundled things be compatibile between modules?

mpetuska commented 3 years ago

Bundled things are invisible outside of the module (as anything without @JsExport) so it's safe to have js-module depend on kotlin modules foo-kt and bar-kt at the same time, even if they both depend (and bundle) on baz-kt.

The only issue with this I can foresee is if baz-kt uses some top-level state (object or top-level val) that can be modified from foo-kt or bar-kt, since both, foo-kt and bar-kt will have a separate embedded copy of those objects, so on js-module changing such state from foo-kt would be invisible to bar-kt.

arkivanov commented 3 years ago

@mpetuska what if baz-kt will have things annotated with @JsExport?

That's probably another question, what we should export? Perhaps just all streams (Observable, Single, Maybe, Completable and all super types), Disposable, etc. No need to export any operators I guess. Perhaps we can just export one module reaktive, no need for other modules in this case! This should allow consumers to subscribe to exported streams and to convert to/from e.g. RxJs, Promise etc.

mpetuska commented 3 years ago

@mpetuska what if baz-kt will have things annotated with @JsExport?

Then copies of those entities will get exposed via both, foo-kt and bar-kt.

That's probably another question, what we should export?

As per @JsExport restrictions, anything that leaks in public domain (argument types, return types, extended super-types) should be annotated.

arkivanov commented 3 years ago

As per @JsExport restrictions, anything that leaks in public domain (argument types, return types, extended super-types) should be annotated.

You probably meant "can be annotated"? I thought we can choose what to export.

mpetuska commented 3 years ago

Yes and no. Have a read through linked docs. You choose what to export, but also you can only export either default types or other @JsExport annotated types. So if you export a function that returns some custom type, that too must be annotated recursivelly.

arkivanov commented 3 years ago

@mpetuska thanks for all the explanations!

mpetuska commented 3 years ago

No worries. Those are more like notes to self as there are just so many moving pieces with kt.js publishing to npm ecosystem :D

gciatto commented 3 years ago

To the best of my understanding, one should js-export the public API of the library, which, in my view, involves all the public types (interfaces, classes, etc) and module functions.

However, I'm currently struggling with these issues:

arkivanov commented 3 years ago

@gciatto-unibo I think what to js-export depends on the library and use cases. For Reaktive I see only the following use case. To create custom Kotlin library/project using Reaktive, which exposes Reaktive streams as public API. E.g. it can be a ViewModel with Observable properties, it can also accept streams as inputs. This library then can be consumed in a JS project, where Reaktive streams should be subscribed or transformed to from RxJs etc. The same situation as with exporting iOS frameworks. So we don't really need to export the utils module, and also don't need to export any operators. Just some public interfaces and classes, and maybe some factory functions.

Also thanks for linking the issues, I will check them.

ankushg commented 3 years ago

You can't go around kotlin dependencies bundling, unfortunately, but you still get fully functional npm modules. And the size is not that big of an issue for node.js, only browser consumers would feel the bloat.

On our end at Quizlet, our primary use-case for Kotlin/JS is actually the browser, so the bloat is definitely felt by us 😅

Our goal is to generate a separate NPM artifact per Gradle module, declare dependencies between modules, and to specifically choose which KMP modules to include on each page. This allows us to keep the bloat to a minimum for our users.

If I add the "-Xir-per-module" compiler option to reaktive on @mpetuska's branch, we get separate JS files in reaktive/build/publications/npm/js each for

This may be out of scope for the Reaktive use case if the primary target is NPM, but for Quizlet's browser use case, I'd love to see a way of taking an "umbrella" module that has several dependencies, using -Xir-per-module, and then packaging each module into a separate NPM artifact.

mpetuska commented 3 years ago

Hmm, I wasn't aware of that flag. I'll tinker with it on npm-publish plugin over the weekend to see if something can be hacked to maintain modularity when assembling for js. But in any case, the bloat will not go away until kotlin.js supports es6 and produces "tree-shakeable" output.

ankushg commented 3 years ago

Sounds good!

To clarify: npm-publish would be much more browser-friendly if we could take an "umbrella" module that has several dependencies, use -Xir-per-module, and package each module into a separate NPM artifact.

Using the Reaktive example, a browser-friendly use case would have these JS files published as four different artifacts:

In terms of consuming Kotlin/JS code in a "regular" JS browser app:

You're absolutely right that Kotlin/Js needs to support ES6 and treeshaking for real bloat reduction 😄

However, even though these files aren't ES6 and tree-shakable, they've at least all been DCEd.

It also lets the consuming app be more granular. It can depend only on @reaktive-utils, transitively pull in a DCE'd version of kotlin stdlb, and avoid pulling in the entire reaktive codebase that it doesn't need.

This also lets the consuming browser app use Webpack's code-splitting to allow users to cache each module separately across pages, so they're reused after being fetched for the first time.

In terms of making this "work"

The tricky things I can identify with how this plays with npm-publish's current approach is that we'd want to

We (at Quizlet) have a lot of really jank gradle scripts that does this for us, but it's extremely specific to our use case and contains a TON of assumptions that definitely don't generalize.

I'd love to work with you to see if there's any way to help!

mpetuska commented 3 years ago

Hmm, just tested out -Xir-per-module and it doesn't seem to do anything for me. How exactly do you set it? Currently I'm setting it in kotlin.js().compilations.all { kotlinOptions.freeCompilerArgs += listOf("-Xir-per-module") }

ankushg commented 3 years ago

Hmm, just tested out -Xir-per-module and it doesn't seem to do anything for me. How exactly do you set it? Currently I'm setting it in kotlin.js().compilations.all { kotlinOptions.freeCompilerArgs += listOf("-Xir-per-module") }

In your branch's reaktive/build.gradle.kts, I added this to the bottom:

tasks.withType<KotlinJsCompile>().configureEach {
    kotlinOptions {
        sourceMap = true
        sourceMapEmbedSources = "always"
        freeCompilerArgs = freeCompilerArgs + listOf(
            "-Xir-per-module"
        )
    }
}

Then, when I run ./gradlew :reaktive:pack those files up in the directory and CLI output.

kinda silly how there are so many different ways to add compiler flags -- not sure why one would work but not the other

mpetuska commented 3 years ago

Right, looks like that option needs to be set only on compileProductionLibraryKotlinJs task and is ignored by regular compileKotlin tasks

arkivanov commented 3 years ago

Thanks @ankushg for such detailed and useful inputs!

arkivanov commented 3 years ago

First of all, thanks everyone who participated in this long conversation! I think we all got a lot of new knowledge, use cases, experience, etc. This was very useful.

I talked to @minaEweida (the author of the issue) in Kotlin Slack and asked about use cases of using Reaktive published to NPM. As I was thinking, the use case is pretty obvious - to create a Kotlin/JS SDK (library) using Reaktive, to publish this SDK to a local NPM registry, and to use this published SDK in a pure JavaScript/Node.js/etc. project.

I also talked to some people from JetBrains, who are working on or related to Kotlin/JS. Here is some information I obtained.

When such a Kotlin/JS module is published in Legacy mode, then all the dependencies should be published to NPM as well. This is because in Legacy mode, everything is published per Gradle module separately, and nothing is bundled. But when a Kotlin/JS module is published in IR mode, then everything is bundled into one final package. As I understood, this is by design.

I also checked the kotlinx.coroutines library - it supports both modes Legacy and IR, but it is published to NPM only in Legacy mode.

The Legacy mode will be deprecated and removed eventually. Libraries like kotlinx.coroutines will not be published in IR mode, which means they will stop being published to NPM at all. The recommended way is to add Kotlin/JS dependencies via Gradle and to publish only the final (top level) Gradle module to NPM. Such a module should export all required public API using @JsExport annotation.

Given the information above, it doesn't make sense to publish Reaktive to NPM, because it is not supposed to be used alone in a JavaScript project. The recommended way is to use Reaktive in a Kotlin/JS project, and to publish the final project to NPM using the IR mode. Closing the issue.