fix: only process function if runtime is node

[NoxHarmonium]

We have a Serverless project that mixes Typescript functions with a Python function that uses a prebuilt package. I know it is a strange combo, but was necessary for various reasons.

We realised that serverless-plugin-typescript was trying to handle our python functions and we were getting runtime errors.

I made a patch that fixed the issue for us and I thought it might be useful to others.

Let me know if I've missed anything or you think it needs more tests or an example project.

I've had a look through the common providers (AWS, GCP, Azure, OpenWhisk) and it looks like the node run-time always starts with the string "node" so it should be safe to detect node projects by converting the runtime property to lowercase and checking if it starts with "node". If you can think of any other edge cases let me know.

If a runtime is not specified at all, it seems like Serverless will default to Node, but the runtime variable is not resolved by the the time we use it so it comes through as undefined. Therefore in the case that the global runtime is undefined I think its safe to assume that the project is a Node project.

[vectorjohn]

oh hey, I didn't think to check existing pull requests before I did the same. I submitted PR #197 (and related issue #198). I'll take a look at this and delete my PR if yours looks better.

[NoxHarmonium]

It's pretty funny that this project has been around for over two years and by coincidence we stumble across the same issue within a week of each other and both submit a PR :laughing:

I've found some differences between our PRs:

• In your PR you also filter the functions in the moveArtifacts step. I missed this (probably because I wasn't using service.package.individually)
• We test different things, my tests are more unit tests for smaller components but yours is more of an integration test passing in the serverless.yml file (good idea!)
• My PR is a bit less noisy because I didn't have to move any files around

I think we should probably merge the two PRs together and take the best from each.

If you're happy I can integrate your changes into this PR. Otherwise we can do it the other way around but I feel like it would be easier to copy and paste your changes into mine because I didn't move any files around so its easier to read the diff. I'd also like to avoid doing that so we don't disrupt the git history as much. I've got a few ideas for how to avoid having to move files around.

Let me know what you think!

[vectorjohn]

Yeah, I'm fine with that. I admit I didn't spend much time on the tests. However, the file move I did was actually because my tests wouldn't run without it. That's why I split it into two commits, but maybe the "right" approach was to do two PRs. The way the plugin class is exported from index.ts by doing export class TypeScriptPlugin and again at the bottom doing module.exports = TypeScriptPlugin, that was causing an error where I could import TypeScriptPlugin in my test, but then at runtime I got the error "TypeScriptPlugin is not a constructor" (or not callable or something). I wasn't sure the details about why that happened other than it was related to the module.exports line, so I just changed it so TypeScriptPlugin had its own file.

So if you take my changes, you'll have to remove my test because it won't run otherwise. Unless you can find a different workaround. I'm fine with that since you have tests too.

[NoxHarmonium]

OK I've pulled in your changes @vectorjohn

I realised that the project was already using lodash so we could use pickBy to simplify the logic where we filter the functions.

It looks like index.ts had a mix of old and new module export format. At the top, the class was exported with export class TypeScriptPlugin which is the equivalent to the old style module.exports = { TypeScriptPlugin }. However the module.exports = TypeScriptPlugin at the bottom of the file blatted that. TypeScript thought it could import the class with the import { TypeScriptPlugin } from '../src/' but only import TypeScriptPlugin from '../src/' would have worked. I changed module.exports = TypeScriptPlugin to export default TypeScriptPlugin which should be the equivalent but not conflict with the export class TypeScriptPlugin statement.

I'll have to do some integration testing before this gets merged though.

[NoxHarmonium]

Looks like I messed up the export syntax. After doing some more reading it turns out that the ES6 equivalent of modules.export = xyz is export = xyz

We need to get that syntax right so that the Serverless CLI can import the plugin properly.

I've pushed up a fix.

If you're interested check out: https://www.typescriptlang.org/docs/handbook/migrating-from-javascript.html#exporting-from-modules https://stackoverflow.com/questions/40294870/module-exports-vs-export-default-in-node-js-and-es6

I've done some integration testing in my dependant project after fixing that issue (with and without package individual) and it seems to work well :+1:

[vectorjohn]

Thanks for the link, that export = xyz is exactly what I was looking for. I had already tested export default and ran into the same problem you did when using the plugin in Serverless.

[KingDarBoja]

@NoxHarmonium @vectorjohn Good job both of your PRs. I think I will merge both on my fork and solve any conflict in order to provide insight if those get merged here.

EDIT Just noticed this PR already merged the one from vectorjohn 💯

[KingDarBoja]

Hi @NoxHarmonium and @vectorjohn

I recently published a new version from my forked repo on npm: @kingdarboja/serverless-plugin-typescript

Please test it as I merged several PRs from this repo in order to solve several issues, including this one.

Cheers!

[NoxHarmonium]

@KingDarBoja Thanks I’ll give it a try. It would be good to use a versioned NPM dependency in our application rather than a git branch. Are you going to be maintaining the fork going forward or is it just a temporary thing to get all the fixes published?

[KingDarBoja]

So far, thinking on maintaining the fork and, at some point, it should become its own repo and not merely a fork as shown on GitHub.

And what do you mean by versioned npm dependency? If it is about the package name, I had to use scoped package name since it is someone works as stated on the MIT License.

Sorry, I am newbie on these things about publishing, it is my first published package 🙈

[NoxHarmonium]

Sorry that was probably badly worded :laughing: I meant its good that you published the fork to NPM, because at the moment we have an application that references this branch directly and pulls this module from git, but now we can use your published fork which is properly versioned and is on NPM rather than git so it won't disappear. Thanks again, I'll let you know how it goes!

[KingDarBoja]

No problem, thanks to you, this PR was helpful 🎖

[mattmeye]

Hello, i have tested @kingdarboja/serverless-plugin-typescript, but get an error in combination with serverless-offline and another function in java. Did you tested this, too?

Error message: "TypeError: Cannot read property 'toUpperCase' of undefined"

[NoxHarmonium]

Hi @mattmeye this PR might not the best place to raise that issue. It might be better to open an issue at https://github.com/KingDarBoja/serverless-plugin-typescript/issues with some more details about how to reproduce the problem.

FWIW I was having that issue on a project and it was a breaking change in the Serverless library and I just needed to update my serverless-offline package (see https://github.com/dherault/serverless-offline/issues/126)