Skip to content

feat: add support for .mjs file output #333

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

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

codingnuclei
Copy link
Contributor

@codingnuclei codingnuclei commented Jul 15, 2022

Hey.

This PR adds outputting .mjs files if the build option platform=neutral.

Currently we can use platform=neutral to output the bundle as esm. The bundle extension however remains .js. This is fine when it is accompanied by a package.json which has "type": "module" set.

However, the `package.json' is explicitly not included in the final zip.

https://github.com/floydspace/serverless-esbuild/blob/master/src/pack.ts#L38

This means that lambdas run the code as CommonJs.

Changing the file type seems a more explicit option (rather than package the package.json)

Considerations:

-platform=neutral now outputs .mjs files

Notes on testing

  • How typing is currently set up, it is hard to change buildOptions in tests. I have therefore casted when added to the overrides. Keeping the variable typed should ensure the type safety.

First PR to serverless-esbuild so let me know if you require anything else.

Partially closes #315 (package.json is no longer required)

Update

Have updated so that .js is the default extension thereby making this a none breaking change.
If you wish to change the extension you can now use the new outputFileExtension which has some validation to keep the user right.

@codingnuclei codingnuclei force-pushed the feature/mjs_file_output branch from e11ec2b to 0080a7b Compare July 17, 2022 22:18
@codingnuclei
Copy link
Contributor Author

PR changed to be more configurable

@floydspace
Copy link
Owner

Hi @codingnuclei , thank you for the careful PR. I think it's a necessary addition.
I'm happy to merge it, but please consider my comments about serverless error

@codingnuclei
Copy link
Contributor Author

codingnuclei commented Jul 18, 2022

Hi @codingnuclei , thank you for the careful PR. I think it's a necessary addition. I'm happy to merge it, but please consider my comments about serverless error

@floydspace This might be me being silly - but I am not seeing any comments?

@floydspace
Copy link
Owner

@codingnuclei oh I was not aware that comments are not visible unless a review is submitted, so it was silly from my side

@codingnuclei codingnuclei force-pushed the feature/mjs_file_output branch from 0080a7b to b799b34 Compare July 18, 2022 11:11
@codingnuclei
Copy link
Contributor Author

@floydspace updated with your comments resolved :)

@codingnuclei codingnuclei requested a review from floydspace July 18, 2022 11:16
Copy link
Owner

@floydspace floydspace left a comment

Choose a reason for hiding this comment

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

@codingnuclei thank you, approved.

@floydspace floydspace merged commit a233a1a into floydspace:master Jul 18, 2022
@github-actions
Copy link

🎉 This PR is included in version 1.32.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Extra file (package.json) not included into final zips
2 participants