Support for JS private instance fields?

[raymond-w-ko]

I'm using shadow-cljs to target a :node-library for AWS Lambda node v14. Recently I tried upgrading to using NPM module maradb @ 3.0.0. I encountered the following build failure:

/home/user/src/project/node_modules/mariadb/lib/connection.js

it was required from
  /home/user/src/project/node_modules/mariadb/promise.js

Errors encountered while trying to parse file
  /home/rko/src/project/node_modules/mariadb/lib/connection.js
  {:line 1352, :column 2, :message "'}' expected"}

It seems it caused by this line: https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/64ca753b7aa1a5d13e8e14799af5bda4271d5ce4/lib/connection.js#L1356 which is using private field syntax.

I tried changing :output-feature-set to :es-next as well as upgrading from node v14 to node v16, but this doesn't help.

Is this something that need Google Closure support, or something needs transpiling support like babel? Or is parsing done inside shadow-cljs and it is something that needs to be added?

[thheller]

Which shadow-cljs version do you use? There is nothing to be done from the shadow-cljs side. This is a Closure Compiler issue. If you are using the latest version of shadow-cljs (also of Closure Compiler) then I guess this isn't supported.

Which target are you compiling for though? mariadb-connector-nodejs shouldn't require any processing of shadow-cljs at all, assuming the final target will run in node anyways? It will most certainly not run in the browser anyways?

[raymond-w-ko]

Which shadow-cljs version do you use?

The latest one: shadow-cljs - server version: 2.17.8 running at http://localhost:3449

Which target are you compiling for though?

:node-library

mariadb-connector-nodejs shouldn't require any processing of shadow-cljs at all, assuming the final target will run in node anyways?

Yeah, it should not. I was just wondering if there was a way to get shadow-cljs to understand it.

It will most certainly not run in the browser anyways?

Correct.

Thanks for narrowing it down, searching some more I found the open issue in Google Closure Compiler for this: google/closure-compiler#2731 . It looks like they will eventually add it, but it is going to take awhile.

Looks like I just have to freezer the version for mariadb-connector-nodejs at 2.x for the foreseeable future.

Thanks. You can close this if you feel like this shouldn't be an open issue.

[thheller]

Did you set anything in your build config that would change how JS is processed? ie. :js-provider? Or how are you requiring the package? This should not be getting processed at all for :node-library? Or are you maybe trying to work on this from a browser-repl?

[raymond-w-ko]

Oops, sorry for missing this the point above.

I have set :js-provider to :shadow, which I understand is not the default.

The reason I did this is because I want as few files as possible. AWS Lambda is susceptible to cold start times, and the last time I checked, things start measurable faster if I pack everything into one file. So we either have to pay a premium to keep instances warm or try to make sure things start as fast as possible (around 1 second).

[thheller]

I recommend not setting :js-provider and instead post-processing the output with something like https://github.com/vercel/ncc. This has much better support for node specific things and likely doesn't have this issue as well.

[raymond-w-ko]

Thank for this recommendation. I'll look into it when I have time.

[raymond-w-ko]

This question is probably a long shot, but do you know if there is a bundler that supports bundling of the output of shadow-cljs while also somehow integrating the source map for debugging crashes?

I tried ncc but it ignores the source map generated by shadow-cljs.

I tried esbuild but it says Source maps with "sections" are not supported, and indeed a crash does not have line information.

[thheller]

You can try :target :npm-module instead of :node-library. That usually works better when integrating with other tools.

[raymond-w-ko]

Doing that didn't help. ncc doesn't seem to want to consider the input file source map. esbuild does not support source maps with sections.

However, I did discover a workaround. Due to the fact that shadow-cljs (ClojureScript?) generated source maps just have one section, it is possible to collapse it into a traditional source map. I created some sample code here to convert into a source map that esbuild supports: https://gist.github.com/raymond-w-ko/fea13a99975af5796e782699209d3b64

This way I was able to use esbuild to bundle everything into one file. However, for now names in source maps aren't supported (evanw/esbuild#1296), meaning that function names are mangled. This can probably be work around with :pseudo-names, although I need to benchmark to see if the increased file matters at all in a serverless context, when the whole thing is zipped anyway, and you are reading the file locally via node. I'm guessing it should not.