Skip to content
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

A LazyImportSplitRender to import views dynamic. #24

Open
elgca opened this issue Nov 27, 2024 · 8 comments
Open

A LazyImportSplitRender to import views dynamic. #24

elgca opened this issue Nov 27, 2024 · 8 comments

Comments

@elgca
Copy link

elgca commented Nov 27, 2024

Usually, our SPA have many pages and rely on different JavaScript libraries. Using SplitRender would cause all dependent projects to be loaded at startup, even if the current page being accessed is a static page. If the JavaScript library is large, it will slow down the startup speed, and some packages have side effects upon import.

import com.raquo.waypoint.SplitRender
val view = SplitRender(Routes.currentPageSignal.debugLog())
  .collect[Login](_ => LoginView)
  .collectStatic(Register)(RegisterView())
  .collectStatic(Documentation)(DocumentationView())
  .collectSignal[Dashboard](s => DemoApp.myApp) 
  .collectStatic(NotFound)({
    if checkAuth.isEmpty then Routes.pushState(Login())
    div(
      a(Routes.navigateTo(Login()), "Go to Login"),
      div(
        a(checkAuth.toString(), Routes.navigateTo(Dashboard("NotFount"))),
      ),
      button(cls := "btn btn-primary", "Go to Dashboard", onClick --> (e => Routes.pushState(Dashboard("NotFount")))),
    )
  })
//  .signalWithLoading(loadingPage)
  .signal

image

In response to this situation, I have designed this SplitRender, which can automatically split page imports, using the inline keyword and js.dynamicImport .As seen in the figure below, the UI5 JavaScript is no longer being loaded. you just need to provide a loadingPageView.

import elgca.core.utils.router.LazyImportSplitRender as SplitRender
//import com.raquo.waypoint.SplitRender
val view = SplitRender(Routes.currentPageSignal.debugLog())
  .collect[Login](_ => LoginView)
  .collectStatic(Register)(RegisterView())
  .collectStatic(Documentation)(DocumentationView())
  .collectSignal[Dashboard](s => DemoApp.myApp) 
  .collectStatic(NotFound)({
    if checkAuth.isEmpty then Routes.pushState(Login())
    div(
      a(Routes.navigateTo(Login()), "Go to Login"),
      div(
        a(checkAuth.toString(), Routes.navigateTo(Dashboard("NotFount"))),
      ),
      button(cls := "btn btn-primary", "Go to Dashboard", onClick --> (e => Routes.pushState(Dashboard("NotFount")))),
    )
  })
  .signalWithLoading(loadingPage)
//  .signal

image

Dependencies are only loaded when the corresponding View is opened.

image

As you can see, the JavaScript has been split into multiple modules, and the effect of the import has been deferred until the first invocation.

image

  var LazyImportSplitRender_this = new $c_Lcom_raquo_waypoint_SplitRender(pageSignal, renderers);
  var evidence$1$proxy1 = new $j_internal$002db28b7af69320201d1cf206ebf28373980add1451.$c_s_reflect_ClassTag$GenericClassTag($j_internal$002db28b7af69320201d1cf206ebf28373980add1451.$d_Lelgca_app_routes_Login.getClassOf());
  var wrapper = new $c_Lelgca_core_utils_router_ImportWrapper(new $j_internal$002db28b7af69320201d1cf206ebf28373980add1451.$c_sjsr_AnonFunction0((() => import("./internal-59c367f071ff9896a8f60ce4c596c625b0fa2259.js").then((($module) => $module.$s_Lelgca_app_JsApp$package$$anon$1__dynamicImport$__O())))));
  var underlying = LazyImportSplitRender_this.collect__F1__s_reflect_ClassTag__Lcom_raquo_waypoint_SplitRender(new $j_internal$002db28b7af69320201d1cf206ebf28373980add1451.$c_sjsr_AnonFunction1(((p) => {

https://github.com/elgca/Waypoint/blob/master/js/src/main/scala-3/com/raquo/waypoint/LazyImportSplitRender.scala

@raquo
Copy link
Owner

raquo commented Nov 28, 2024

That's a good problem to solve, thanks for the head start!

I see how you did it, and that makes sense. Basically it's functional sugar for returning js.dynamicImport(view) instead of view for each case, and then flattening the signal. We need compile-time execution for this because js.dynamicImport is Scala.js magic, not a regular function that we can trivially compose (but in Scala 3 we can abstract over it with inline).

Although the LazyImportSplitRender implementation will work for its purpose, one issue with it is that it requires every collect* case to be loaded dynamically. We can let users control that by providing both dynamic and non-dynamic versions of every collect* operator, but that will grow the number of such operators quite a bit.

I feel like the solution to this could be a bit higher up, in Airstream. For example, perhaps we should implement EventStream.dynamicImport(view) and Signal.dynamicImport(view) using the same inline trick that you used here, then we could do something like:

import com.raquo.waypoint.*
import com.raquo.laminar.api.L.*

val view = SplitRender[Page, Signal[Option[View]]](Routes.currentPageSignal.debugLog())
  .collect[Login](_ => Val(Some(LoginView)))
  .collectStatic(Register)(Val(Some(RegisterView())))
  .collectStatic(Documentation)(Signal.dynamicImport(DocumentationView()))
  .collectSignal[Dashboard](s => Signal.dynamicImport(DemoApp.myApp)) 
  .flattenSwitch {
    case None => Val(div("Loading..."))
    case Some(viewSignal) => viewSignal
  }
  
div(
  child <-- view
)

In this example, login and registration pages would be included in the same JS file, whereas documentation and dashboard pages would be loaded dynamically. I haven't compiled this, so there may be typos, but I'm pretty sure it should work.

I think I prefer this approach over extending SplitRender for a few reasons:

  • Works for all Airstream things, not just SplitRender
  • Especially because we are getting a Scala 3 replacement for SplitRender, see Add SplitByTypes observables Airstream#116 and Fix: SplitByType macros Airstream#133
  • Is explicit about the need to wrap and flatten non-dynamicImport views. Because flattening is not always consequence-free due to it creating a transaction boundary.
  • More flexible, including with loading screens – can provide different screens for each collect* case if needed

But yes, my proposal is a bit more verbose. But more explicit and more universal. Tradeoffs...

In the future, if / when I implement raquo/Laminar#157, we may be able to simply pass child <-- EventStream.dynamicImport(DocumentationView()) to e.g. collectStatic, and would not need a flattenSwitch at all. But that is a while away.

I need some time to think this over, and work on other things, but this problem is definitely something that should be solved in Waypoint. Let's keep this issue open to track it. Adding Signal.dynamicImport to Airstream is pretty easy, so perhaps I'll start with that (not right now, but possibly in time for Airstream 17.2 release).

For 17.2 there's a slight complication that EventStream.fromJsPromise / Signal.fromJsPromise accept the promise by-value, instead of delaying its evaluation until the stream / signal is started, but I can still implement a lazy EventStream/Signal.dynamicImport wrapper with a flatMapSwitch inside. I may change those arguments to be by-name in 18.0, haven't decided on that yet.

@elgca
Copy link
Author

elgca commented Nov 29, 2024

Especially because we are getting a Scala 3 replacement for SplitRender, see raquo/Airstream#116 and raquo/Airstream#133

This means that in the future, Airstream's functionality will be used directly to replace SplitRender?

For 17.2 there's a slight complication that EventStream.fromJsPromise / Signal.fromJsPromise accept the promise by-value, instead of delaying its evaluation until the stream / signal is started

This is the reason why I used ImportWrapper to wrap js.dynamicImport. EventStream/Signal.dynamicImport is a good idea; how are you planning to define their return types, as Signal[A] or Signal[Option[A]]? Both have their own use cases:

div(child <- EventStream.dynamicImport(view()))
// and
div(child <- EventStream.dynamicImportOpt(view()).map(_.getOrElse(div(a skeleton element)))

Although the LazyImportSplitRender implementation will work for its purpose, one issue with it is that it requires every collect* case to be loaded dynamically.

LazyImportSplitRender is implemented as you said, because what I had in mind was to simply replace it without adjusting the code that uses it.

import elgca.core.utils.router.LazyImportSplitRender as SplitRender

@raquo
Copy link
Owner

raquo commented Nov 30, 2024

This means that in the future, Airstream's functionality will be used directly to replace SplitRender?

Yes. But, the new functionality is macro-based, so it's only for Scala 3. We won't be implementing it for Scala 2.

So, I'm not going to throw out SplitRender yet, it will stay for now.

If there's enough interest, I may even move SplitRender into Airstream, and rename / rearrange the methods so that the API looks pretty much like the new macros do, (except it will remain without exhaustivity checks – those are not possible without macros). But, only if people ask for it – as it stands, I have higher priority tasks in the queue.

how are you planning to define their return types, as Signal[A] or Signal[Option[A]]?

Same as EventStream.fromJsPromise and Signal.fromJsPromise already do – EventStream.fromDynamic would return EventStream[A], and Signal.fromDynamic would return Signal[Option[A]] by default, or, if you provide the initial value as the second argument, then you'll get Signal[A].

Whenever you have an stream: EventStream[A] and you want to provide an initial value like div(a skeleton element), you can just say stream.startWith(initialValue), and you'll get a Signal[A] whose value is initialValue until the stream emits its first event.

@raquo
Copy link
Owner

raquo commented Dec 2, 2024

@elgca I tried adapting your code to make Signal.dynamicImport, but I can't get it to work.

Let's ignore the URL routing part for now – if I just take this part of your code:

sealed trait SignalWrapper[View] {
    def signal: Signal[View]
  }

  object SignalWrapper {
    @nowarn
    inline def apply[View, LazyImport <: Boolean](view: View): SignalWrapper[Option[View]] = {
      if constValue[LazyImport] then new LazyImportWrapper(() => js.dynamicImport(view))
      else new StaticImportWrapper(Val(Some(view)))
    }
  }

  class LazyImportWrapper[View](dynamicImport: () => js.Promise[View]) extends SignalWrapper[Option[View]]:
    private var innerSignal: js.UndefOr[Signal[Option[View]]] = js.undefined

    def signal: Signal[Option[View]] = {
      if (innerSignal.isEmpty) {
        innerSignal = Signal.fromJsPromise(dynamicImport())
      }
      innerSignal.get
    }
  end LazyImportWrapper

  class StaticImportWrapper[View](viewVal: Val[Option[View]]) extends SignalWrapper[Option[View]]:
    def signal: Signal[Option[View]] = viewVal
  end StaticImportWrapper

And let's say I want to load this module dynamically:

import com.raquo.laminar.api.L.*

object FOO {

  def apply() = div(
    "HELLO FOO",
    "FOO",
    "FOO",
    "FOOOOOOOO"
  )
}

And I try to use SignalWrapper to load it dynamiclly:

val bus = new EventBus[Unit]
div(
  div(
    "Click here",
    onClick.mapToUnit --> bus,
    // this works:
    // child.maybe <-- bus.events.flatMapSwitch(_ => LazyImportWrapper(() => js.dynamicImport(FOO())).signal),
    // this does not work:
    child.maybe <-- bus.events.flatMapSwitch(_ => SignalWrapper[HtmlElement, true](FOO()).signal),
  )
)

That does not work. Gives me a "stub" exception that originates over here

On the other hand, if I replace SignalWrapper usage with LazyImportWrapper(() => js.dynamicImport(FOO())).signal, then everything works as expected.

No matter what I try, I seem to be unable to hide the js.dynamicImport call inside SignalWrapper. But, it works for you? Can you please try my code snippets in your browser? I wonder what's different, why it works for you but not for me... Do you have any special build config in your project? I was testing these snippets in https://github.com/raquo/laminar-full-stack-demo

Minimizing the code, with your pattern, I would expect this to work:

  @nowarn inline def dynamicImportSignal[View](view: View): Signal[Option[View]] = {
    Signal.fromValue(()).flatMapSwitch(_ => Signal.fromJsPromise(js.dynamicImport(view)))
  }

But, it does not – gives the same "stub" exception. I tried that on Scala 3.3.4, 3.6.1, Scala.js 1.16.0 and 1.17.0, in Chrome and Firefox.

@elgca
Copy link
Author

elgca commented Dec 2, 2024

I add some magic to LazyImportSplitRender, and it can now control the switching between dynamic and non-dynamic without adding any operation functions. now, it's good enough for me.

import elgca.core.utils.router.LazyImportSplitRender as SplitRender
//import com.raquo.waypoint.SplitRender
val view = SplitRender(Routes.currentPageSignal.debugLog())
 .staticImport
  .collect[Login](_ => LoginView)
  .collectStatic(Register)(RegisterView())
.dynamicImport
  .collectStatic(Documentation)(DocumentationView())
  .collectSignal[Dashboard](s => DemoApp.myApp) 
.staticImport
  .collectStatic(NotFound)({
    if checkAuth.isEmpty then Routes.pushState(Login())
    div(
      a(Routes.navigateTo(Login()), "Go to Login"),
      div(
        a(checkAuth.toString(), Routes.navigateTo(Dashboard("NotFount"))),
      ),
      button(cls := "btn btn-primary", "Go to Dashboard", onClick --> (e => Routes.pushState(Dashboard("NotFount")))),
    )
  })
  .signalWithLoading(loadingPage)

https://github.com/elgca/Waypoint/blob/master/js/src/main/scala-3/com/raquo/waypoint/LazyImportSplitRender.scala

@elgca
Copy link
Author

elgca commented Dec 2, 2024

@raquo
I will give the issue you mentioned a try, a bit later on.

@elgca
Copy link
Author

elgca commented Dec 2, 2024

@raquo
I tried it in laminar-full-stack-demo, it works. it is strange. In laminar-full-stack-demo, I didn't modify the compilation configuration. Is your SignalWrapper placed in a separate Scala file, I'm not sure if it has any impact. Or perhaps it's the magic of constValue[LazyImport] that's causing it.

image

I tried it in the Edge browser. versions is :

object Versions {

  val Scala_3 = "3.3.1"

  // -- Shared --

  val JsoniterScala = "2.24.0"

  val Borer = "1.14.0"

  // -- Frontend --

  val Laminar = "17.1.0"

  val LaminarShoelace = "0.1.0"

  val Waypoint = "8.0.0"

  val UI5 = "1.24.0"

  // -- Backend --

  val CatsEffect = "3.5.2"

  val Http4s = "0.23.23"

  val Log4Cats = "2.7.0"

  val Logback = "1.4.12"

  val Phobos = "0.21.0"

  val ScalaJsStubs = "1.1.0"

  // -- Testing --

  // val JsDom = "20.0.3"

}

Can you create a branch in the laminar-full-stack-demo? I can try it later.

@raquo
Copy link
Owner

raquo commented Dec 8, 2024

@elgca Thanks for checking. It appears we may have found a Scala bug.
I filed the issue with a reproduction here: scala/scala3#22162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants