Conversation
webpro
left a comment
webpro
left a comment
There was a problem hiding this comment.
Thanks Bryan! Happy to merge the SST plugin, but I do have a few remarks.
| const entry = [ | ||
| "sst.config.{js,cjs,mjs,ts}", | ||
| "{handlers,lambdas}/*.{js,cjs,mjs,ts}", | ||
| "src/{handlers,lambdas}/*.{js,cjs,mjs,ts}", |
There was a problem hiding this comment.
Do we have any docs/resources that indicate these could be considered defaults locations?
| import type { IsPluginEnabled, Plugin } from "../../types/config.js"; | ||
| import { hasDependency } from "../../util/plugin.js"; | ||
|
|
||
| // link to sst docs |
There was a problem hiding this comment.
Please add a relevant link to SST docs re. config.
| // console.log('issues', issues); | ||
|
|
||
| assert.partialDeepStrictEqual(issues, { | ||
| unlisted: { |
There was a problem hiding this comment.
The idea that most plugins follow is to have zero issues left, so we only need to check all issue counters are zero, e.g.:
assert.deepEqual(counters, {
...baseCounters,
processed: 4,
total: 4,
})In case there are still issues left we bump the relevant counter and use additional separate assertions for issues like so:
assert(issues.devDependencies['package.json']['some-unused-dep']);| const cwdEmptyConfig = resolve('fixtures/plugins/sst/empty-config'); | ||
| const cwdOneHandler = resolve('fixtures/plugins/sst/one-handler'); | ||
| const cwdOneHandlerInSrcLambdas = resolve('fixtures/plugins/sst/one-handler-in-lambdas'); | ||
| const cwdOneHandlerInSrcHandlers = resolve('fixtures/plugins/sst/one-handler-in-src'); |
There was a problem hiding this comment.
Maybe good to move each cwd to each test context individually to hold them together.
| @@ -0,0 +1,4 @@ | |||
| export type PluginConfig = { | |||
| plugins?: string[]; | |||
| entryPathsOrPatterns?: string[]; | |||
There was a problem hiding this comment.
This is probably coming from the template, any chance it could reflect the actual SST config interface? If there's nothing interesting from the plugin perspective it could be left empty or even removed.
| }); | ||
|
|
||
| // console.log('issues', issues); | ||
| // console.log('counters', counters); |
There was a problem hiding this comment.
Please remove console.log statements
webpro
force-pushed
the
main
branch
2 times, most recently
from
e8120d3 to
93c724a
Compare
|
@BryanCrotazGivEnergy I've been working on support for a new |
|
Sounds good, I'll take a look at #1005 |
|
slogging through other pnpm issues, once those are sorted and we can build and deploy I'll return to this plugin and 1005 |
|
Closing this in favor of #1005. |
2 participants
Added a v1 sst plugin.
Loads lambda handlers from:
/handlers/*
/lambdas/*
/src/handlers/*
/src/lambdas/*
Work left for a v2 plugin to read the sst lambda setup and explicitly load the lambda handlers as entry points