-
Notifications
You must be signed in to change notification settings - Fork 359
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
feat: automatic create tsconfig for editor support on functions #6036
base: main
Are you sure you want to change the base?
feat: automatic create tsconfig for editor support on functions #6036
Conversation
6355b77
to
631e53c
Compare
// "extends": "../tsconfig.json", /** If you want to share configuration enable the extends property (like strict: true) */ | ||
"compilerOptions": { | ||
"noEmit": true /** This tsconfig.json is only used for type checking and editor support */, | ||
"module": "ESNext", |
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.
I don't think we need to set this or moduleResolution
anymore? I think all we need is allowImportingTsExtensions
+ noEmit
for the .ts
extensions, and then checkJs
+ allowJs
for the JavaScript files?
*/ | ||
export async function checkTsconfigForV2Api(config) { | ||
// if no functionsDir is specified or the dir does not exist just return | ||
if (!config.functionsDir || !existsSync(config.functionsDir)) { |
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.
Have you considered moving this logic into the FunctionsRegistry
(like we did with
cli/src/lib/functions/registry.mjs
Line 181 in f8a9703
this.checkTypesPackage() |
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.
sure but then we can't run it on the create command.
|
||
try { | ||
const require = createRequire(config.functionsDir) | ||
require.resolve('@netlify/functions') |
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.
I stole this from you and it's already been shipped 😬
cli/src/lib/functions/registry.mjs
Line 181 in f8a9703
this.checkTypesPackage() |
@lukasholzer is this PR still relevant, or should we close/draft? |
@sarahetter yes but sadly never had time to finish it up |
Fixes https://github.com/netlify/pod-dev-foundations/issues/595
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes the editor support for the new functions V2 API. By that we enforce that the editor is in sync with the build values
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)