feat: added typescript lambda support #2
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2 +/- ##
==========================================
- Coverage 98.9% 98.52% -0.38%
==========================================
Files 3 3
Lines 91 68 -23
Branches 9 9
==========================================
- Hits 90 67 -23
Misses 1 1
Continue to review full report at Codecov.
|
lib/commands/serve.js
Outdated
| @@ -11,6 +11,8 @@ const clearModule = require('clear-module') | |||
|
|
|||
| const { createError } = micro | |||
|
|
|||
| const lamdbaMap = new Map() | |||
There was a problem hiding this comment.
I created a map which will cache imported lambdas upon usage. This was necessary to fix a problem I encountered when requiring a lambda that was importing and using firebase.
There was a problem hiding this comment.
Can you provide me with the full firebase related error? We can't really allow for module cache reusing for this would break the concept of self-enclosed, no-shared-context of lambda executions. I tried something with child processes, but it didn't work so far. Maybe we can clear a broader module scope on clearModule.match(...) to ensure anything needed by the lambda gets cleared out and re-loaded...
There was a problem hiding this comment.
This was the error output of the second time you'd request a lambda using firebase:
Error: Module did not self-register.
at Object.Module._extensions..node (internal/modules/cjs/loader.js:718:18)
at Module.load (internal/modules/cjs/loader.js:599:32)
at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
at Function.Module._load (internal/modules/cjs/loader.js:530:3)
at Module.require (internal/modules/cjs/loader.js:637:17)
at require (internal/modules/cjs/helpers.js:20:18)
at Object.<anonymous> (/[REDACTED]/node_modules/grpc/src/grpc_extension.js:32:13)
at Module._compile (internal/modules/cjs/loader.js:689:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
at Module.load (internal/modules/cjs/loader.js:599:32)
Not really helpful. My only quick thought was that the module being required multiple times was breaking some firebase internal module relationships.
Maybe yes, a broader clean up of modules is the key.
There was a problem hiding this comment.
@Meesayen do you mind to test your firebase related code using branch fs/broader-cache-cleanup?
There was a problem hiding this comment.
Sorry, I couldn't have a look at this lately.
Here's what I'll do, I'll get rid of the caching part on this PR, and unblock the basic typescript support. The firebase issue can wait (firebase doesn't work when deployed on Now anyway...)
lib/commands/serve.js
Outdated
| @@ -32,7 +34,7 @@ class ServeCommand extends Command { | |||
| const { builds = [], routes = [] } = require(nowConfigPath) | |||
|
|
|||
| // find all node lambdas | |||
| const nodeBuilds = builds.filter(({ use }) => use === '@now/node') | |||
| const nodeBuilds = builds.filter(({ use }) => /^@now\/node@?/.test(use)) | |||
There was a problem hiding this comment.
At the moment, Typescript support on Now v2 is only possible via the canary version of @now/node, hence the check for the optional @ (i.e. @now/node@canary will be the builder for some time)
lib/commands/serve.js
Outdated
| ) | ||
| let pathname = urls | ||
| .map( | ||
| url => |
There was a problem hiding this comment.
this can probably be simplified, but in short, we only match Typescript lambdas if they end with .ts explicitly, when requested. Either manually or via a routes alias in now.json.
|
Thank you for this guys, I'm looking forward to using this new feature ! |
| typeof entrypoint === 'string' ? require(entrypoint) : entrypoint | ||
|
|
||
| // Supporting both Typescript and ESM type of imports | ||
| if (lambda.default) lambda = lambda.default |
There was a problem hiding this comment.
it's a bit sloppy, as it can break when importing a CJS module such as:
module.exports = {
default: {...},
fn() {}
}
but since we're dealing with Lambdas, I'd expect that each module will only export exactly one function.
This adds support for Typescript lambdas via a new bin
now-we-test-tswhich automatically injectsts-node/registeras a node require hook.ts-nodeandtypescriptwill be necessary to run.tslambdas, but they have been added as peerDependencies in case the user doesn't need them.I'll leave some comments on the changes to help the reviews.
Fixes: #1