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

ES Module support #246

Open
hugo-vrijswijk opened this issue Jul 18, 2022 · 19 comments
Open

ES Module support #246

hugo-vrijswijk opened this issue Jul 18, 2022 · 19 comments

Comments

@hugo-vrijswijk
Copy link

Native ES modules are finally supported in all current LTS NodeJS versions 🎉. More and more Node libraries are moving to ES modules. ES modules can use a top-level import for CommonJS, but CommonJS cannot import ES modules with top-level imports. This means libraries still using CommonJS risk getting left behind in the Node ecosystem.

Feral currently uses a dynamic export update to export the handler function. From what I could see, this is done to load resources only once instead of every time the lambda is invoked. However, ES modules do not support dynamic export updates. All exports have to be there as export statements. The recommended way for the resource use-case is by using top-level awaits.

To keep Node.js library interop and keep the library modern, it would be ideal for Feral to also support building as a ES Module. I'm not sure if it is possible to instruct Scala.JS to emit different code based on the module setting, or perhaps a different entrypoint should be available for ES Module users.

@armanbilge armanbilge added this to the 0.1.0 milestone Jul 18, 2022
@armanbilge
Copy link
Member

armanbilge commented Jul 18, 2022

Thanks for raising this issue, this is an important point and something to try and address before 0.1.0.

Feral currently uses a dynamic export update to export the handler function. From what I could see, this is done to load resources only once instead of every time the lambda is invoked

Indeed I'm not thrilled about the current encoding. Actually I settled on it in desperation for the friendliest UX. Without it, a user would have to manually override a method in their lambda and add a @JSExport annotation to it. Hopefully this would probably work with ESModules as well, but I have yet to investigate that. If you have any ideas for this I would greatly appreciate it 🙏

IMO the biggest issue for Scala.js and ESModules is lack of GCC optimization, which makes a huge difference especially for Typelevel libs which have several layers of abstraction.

@hugo-vrijswijk
Copy link
Author

hugo-vrijswijk commented Jul 18, 2022

I think you need JSExportTopLevel to export the handler function. Personally, I wouldn't be against a JSExportTopLevel annotation if it is well-documented on how to set up. Currently, what is exported is also a bit 'magical', where a JSExportTopLevel would be more explicit (and support multiple entrypoints). But this is coming from someone with a good amount of Node experience.

JSExportTopLevel also translates to export statements in emitted JS. Perhaps something with a Deferred.unsafeRunAsync and Deferred.get in the lambda handler could keep the current Resource behaviour working?

@armanbilge
Copy link
Member

Yes, perhaps the trade-off is not worth it. But it's a much less fun experience than the JVM side :) thanks for your input.

Perhaps something with a Deferred.unsafeRunAsync and Deferred.get in the lambda handler could keep the current Resource behaviour working?

Indeed this is already how it's working :) the design choice was really only for UX, not implementation :)

private[feral] final lazy val setupMemo: IO[Setup] = {
val deferred = Deferred.unsafe[IO, Either[Throwable, Setup]]
setup
.attempt
.allocated
.flatTap {
case (setup, _) =>
deferred.complete(setup)
}
.unsafeRunAndForget()(runtime)
deferred.get.rethrow

@hugo-vrijswijk
Copy link
Author

Ah, I might've clicked through the sources too quickly and thought I came up with my own brilliant idea 😉. Perhaps making the handlerFn protected instead of private could allow users to implement their own exports? Or something like program: IO[UndefOr[Any]] to expose the lambda while staying inside IO. Albeit also not the cleanest solution

object handler extends IOLambda[ApiGatewayProxyEventV2, ApiGatewayProxyStructuredResultV2] {

  override def handler = ???

  @JSExportTopLevel("handler")
  def main() = handlerFn

  @JSExportTopLevel("handlerIO")
  def main() = program.unsafeToPromise()(runtime)

}

@armanbilge
Copy link
Member

armanbilge commented Jul 18, 2022

At least for CommonJS, you have to export a function with a very specific signature, a def main() like that won't work. This means leaking facade types into userland, which sucks.

@armanbilge
Copy link
Member

@hugo-vrijswijk basically something like this:

private[lambda] trait IOLambdaPlatform[Event, Result] {
this: IOLambda[Event, Result] =>
// @JSExportTopLevel("handler") // TODO
final def handler(event: js.Any, context: facade.Context): js.Promise[js.Any | Unit] =
(for {
setup <- setupMemo
event <- IO.fromEither(decodeJs[Event](event))
result <- apply(event, Context.fromJS(context), setup)
} yield result.map(_.asJsAny).orUndefined).unsafeToPromise()(runtime)
}

Except we can't put @JSExport in an abstract class, so the user needs to override that method and do it. It's really terrible tbh, since the facades leak and they also need to delegate to the super implementation ...

@hugo-vrijswijk
Copy link
Author

hugo-vrijswijk commented Jul 18, 2022

Maybe this would work?

private[lambda] trait IOLambdaPlatform[Event, Result] {
  this: IOLambda[Event, Result] =>

  final type HandlerFn = js.Function2[js.Any, facade.Context, js.Promise[js.Any | Unit]]
  
  protected lazy val handlerFn: HandlerFn = ...
...
}
object MyMain extends IOLambda.Simple[KinesisStreamEvent, INothing] {

  override def apply(event: KinesisStreamEvent, context: Context[IO], init: Init): IO[Option[INothing]] = ???

  @JSExportTopLevel("handler")
  val handlerMain: HandlerFn = handlerFn
}

Note the use of val instead of def

This would export a function, and only expose the handler type, while keeping the actual facade types private

@armanbilge
Copy link
Member

I actually think it doesn't, but I could be wrong. IIRC the problem is that Scala.js exports vals with setters and getters, unlike how it exports a function.

@armanbilge
Copy link
Member

Yup, won't work:

//> using scala "2.13.8"
//> using platform "js"
//> using jsModuleKind "commonJS"

import scala.scalajs.js
import scala.scalajs.js.annotation._

object Lambda {

  @JSExportTopLevel("fnVal")
  val fnVal: js.Function1[String, String] = _.reverse

  @JSExportTopLevel("fnDef")
  def fnDef(x: String): String = x.reverse


  def main(args: Array[String]): Unit = ()
}
Object.defineProperty(exports, "fnVal", {
  "get": (function() {
    return $t_LLambda$__fnVal
  }),
  "configurable": true
});
exports.fnDef = (function(arg) {
  var prep0 = $as_T(arg);
  return $m_LLambda$().fnDef__T__T(prep0)
});

@bpholt
Copy link
Member

bpholt commented Jul 18, 2022

Kind of spitballing here, but we've talked about creating an sbt plugin for Feral to help with deploys, etc. What if we had a plugin that somehow generated the code that was needed?

@hugo-vrijswijk
Copy link
Author

What about the fnVal doesn't work? It is exported as a JS getter, which is the function itself:

node
Welcome to Node.js v16.15.0.
Type ".help" for more information.
> const l = require('./Lambda.js')
undefined
> l.fnVal
[Function: $t_LLambda$__fnVal]
> l.fnVal("foo")
'oof'

@armanbilge
Copy link
Member

Kind of spitballing here, but we've talked about creating an sbt plugin for Feral to help with deploys, etc. What if we had a plugin that somehow generated the code that was needed?

Ah, that's not a bad idea! Definitely worth exploring :)

What about the fnVal doesn't work? It is exported as a JS getter, which is the function itself:

I could be wrong, but I don't think it exports things the way AWS wants them. I remember having problem with this in the past. Regardless of how it behaves in the console, you can see the encoding is obviously different.

@armanbilge
Copy link
Member

This is the integration test I wrote at the time:

if (typeof require('./target/scala-2.13/npm-package/index.js').mySimpleHandler === 'function')
process.exit(0)
else
process.exit(1)

@armanbilge armanbilge mentioned this issue Jul 20, 2022
@armanbilge armanbilge modified the milestones: 0.1.0, v0.2.0 Jul 20, 2022
@hugo-vrijswijk
Copy link
Author

hugo-vrijswijk commented Jul 22, 2022

I did some testing with exports and running the lambda's, and it looks like exporting a val js.Function works the same as exporting a def.

Scala.js emits the function as a JS getter, which AWS Lambda can call just fine as a function (if emitted properly). This works the same when exporting as CommonJS or as an ES module.

  @JSExportTopLevel("fnVal")
  val fnVal: js.Function1[js.Object, Unit] = x => println("fnVal" + js.JSON.stringify(x))

  @JSExportTopLevel("fnDef")
  def fnDef(x: js.Object): Unit = println("fnDef" + js.JSON.stringify(x))

fnVal:
image
fnDef:
image

There is 1 caveat: it only works when the val is exported as a js.Function. If it is exported without (e.g. val fnVal = (x: js.Object) => println("fnVal" + js.JSON.stringify(x))) then a Scala.js wrapper will be emitted around the export:

image

But with the js.Function1 annotation:

image

I don't know if letting the user export their own handler is still the desired way to go vs a sbt plugin that generates some code, but the option is open and working, it looks like. Upside is being able to export multiple handlers, downside I guess is it's slightly more technical to set up. It would also no longer need scalaJSUseMainModuleInitializer := true which is more efficient for bundlers and tree-shakers

@armanbilge
Copy link
Member

@hugo-vrijswijk thank you so much for doing that investigation! That is very good news then, much appreciated. I think we have some good options, and there are many advantages to exports as you point out.

I'm happy to take a PR that starts making these changes :)

@armanbilge
Copy link
Member

Btw, I remembered when I had problem with exporting the val and the getter-based encoding. You can actually see that exact change in ChristopherDavenport/js-test@639cdda. This was back in the early days when I was first working on a serverless example for http4s.js. The problems I had were when testing with AWS SAM locally, so either Amazon has improved/fixed things since then or its inconsistent with the production runtime.

@armanbilge armanbilge removed this from the v0.2.0 milestone Jul 21, 2023
@armanbilge
Copy link
Member

I found new motivation to pursue ES6 modules: we can graduate from our janky initialization procedure by using a top-level await to only export the handler after we've asynchronously acquired the resources.

@hugo-vrijswijk
Copy link
Author

I feel like I looked at that before, but is it possible to get Scala.js to emit a top-level await (or any await?).
Also, I should probably pick this PR up again 😅

@armanbilge
Copy link
Member

but is it possible to get Scala.js to emit a top-level await

Oh, I forgot to link the issue I opened.

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

3 participants