Add ESM support for Node 14.#34
Conversation
ESM is a special flag. It is not implied by "--all" or "--selftest" and is tested separately. Turning the flag on puts the application in "module mode" by setting the "type" flag in `package.json`. `boltzmann.js` then expects to load all application files (middleware, handlers, etc.) via `import()`. The upside: it works! Import syntax makes handlers definition much nicer by baking the export status into the definition of the handler function. This also puts all files into strict mode by default, which is good for usabiliy and performance. The downside is that it forks exports and imports in all of our generated files. I think, medium-to-long-term, it's worthwhile: ESM is the future of JS, t and Node 14 forces the issue. Ultimately, we should default to ESM. HOWEVER, this complicates hot-reloading slightly: it means we end up having to build it twice (or have one mode that supports it, and one that doesn't.)
ceejbot
left a comment
ceejbot
left a comment
There was a problem hiding this comment.
This will create maintenance and testing problems if we land it, but I am assuming we have to take them on because we'll end up here eventually when Node ends up in the ESM future. I do not know what any prospective users of boltzmann would prefer to use. It's possible we should ask some.
| Some(Some(Flipper::Off)) => false, | ||
| None => self.esm.unwrap_or(false) | ||
| }; | ||
|
|
There was a problem hiding this comment.
I was wondering how you'd short-circuit that above mess in the cast lambda. I can't decide if this complexity is necessary and the point of the tool, or if it should be abstracted away.
There was a problem hiding this comment.
(I lean towards "necessary", tbh.)
| // {% if esm %} | ||
| import { createRequire } from 'module' | ||
| import esMain from 'es-main' | ||
| const require = createRequire(import.meta.url) |
| import { fileURLToPath } from 'url' | ||
| import { dirname } from 'path' | ||
|
|
||
| const __filename = fileURLToPath(import.meta.url) |
There was a problem hiding this comment.
Because urls are a sensible way to describe file paths in server runtime.
There was a problem hiding this comment.
JS is reasserting itself here: "we are a web language, not (exclusively) a server language."
| } = {}) { | ||
| return next => { | ||
| return async next => { | ||
| reachability = { ...reachability, ...await extraReachability } |
There was a problem hiding this comment.
oof, yes, gotta await all these...
| export async function greeting(/** @type {Context} */ context) { | ||
| return `hello ${context.params.name}` | ||
| } | ||
| // {% else %} |
| }] | ||
| {%- endif %} | ||
| ] | ||
| // {% else %} |
There was a problem hiding this comment.
Exports do get cleaner! Not much else does.
2 participants
ESM is a special flag. It is not implied by "--all" or "--selftest"
and is tested separately.
Turning the flag on puts the application in "module mode" by setting
the "type" flag in
package.json.boltzmann.jsthen expects to loadall application files (middleware, handlers, etc.) via
import().The upside: it works! Import syntax makes handlers definition much nicer
by baking the export status into the definition of the handler function.
This also puts all files into strict mode by default, which is good for
usabiliy and performance.
The downside is that it forks exports and imports in all of our generated
files. I think, medium-to-long-term, it's worthwhile: ESM is the future of JS,
t and Node 14 forces the issue. Ultimately, we should default to ESM. HOWEVER,
this complicates hot-reloading slightly: it means we end up having to build
it twice (or have one mode that supports it, and one that doesn't.)
NB: stackman support is broken for now – I've opened a PR that fixes the ESM-induced regression.