-
Notifications
You must be signed in to change notification settings - Fork 352
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
fix: also run blobs on ntl serve
#6183
Conversation
src/commands/serve/serve.mts
Outdated
@@ -71,7 +71,7 @@ const serve = async (options, command) => { | |||
// Netlify Build are loaded. | |||
await getInternalFunctionsDir({ base: site.root, ensureExists: true }) | |||
|
|||
let settings = /** @type {import('../../utils/types.js').ServerSettings} */ ({}) | |||
let settings = /** @type {import('../../utils/types.js').ServerSettings} */ {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Since you're touching this line, could you use the actual type instead of the JSDoc now that this is TypeScript?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 9eea77d
src/commands/serve/serve.mts
Outdated
// @ts-expect-error TS(2571) FIXME: Type '{}' is not assignable to type 'ServerSettings'. | ||
let settings: ServerSettings = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get rid of the ts-expect-error
ugliness if you change L84 to return exit(1)
(GitHub won't let me add a suggestion for that line).
// @ts-expect-error TS(2571) FIXME: Type '{}' is not assignable to type 'ServerSettings'. | |
let settings: ServerSettings = {} | |
let settings: ServerSettings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smart! done in 2719485
Noticed that we're only running blobs for
ntl dev
, but not forntl serve
. That's probably not right! This PR also starts the blob server duringntl serve
.