Skip to content
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: Allow typescript handler #176

Closed

Conversation

piaverous
Copy link

This PR addresses the following issue: #85

It allows the user to set a global config parameter tsHandler: true, to generate a warmup function written in Typescript instead of Javascript

As mentioned in the issue, serverless/serverless-plugin-typescript#79 should have helped fixing this issue. However, in case of a repo whose code is fully typescript, and where tsconfig.json contains allowJs: false, you would still get the following error

{ file: undefined,
  start: undefined,
  length: undefined,
  messageText: 'File \'_warmup/index.js\' has unsupported extension. The only supported extensions are \'.ts\', \'.tsx\', \'.d.ts\'.',
  category: 1,
  code: 6054,
  reportsUnnecessary: undefined }

It was therefore not enough, and having the handler written in JS still caused an issue.

@juanjoDiaz
Copy link
Owner

HI @piaverous ,

Thanks for the contribution.
I'm still pretty certain that we shouldn't support more than one handler.
I can't find any mention to allowJs in the issue that you reference.
In any case, it seems clear to me that the right behaviour of the typescript plugin would be to just ignore JS handlers (so the TS settings should be irrelevant). So I'd still say that the problem is on the serverless-plugin-typescript.

Wdyt?

@juanjoDiaz
Copy link
Owner

Closing for the reasons stated above.

@juanjoDiaz juanjoDiaz closed this Oct 10, 2019
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggested code work for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants