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

Support mjs files as entrypoints #225

Closed
SaschaSchwarze0 opened this issue Jul 13, 2022 · 8 comments · Fixed by #570
Closed

Support mjs files as entrypoints #225

SaschaSchwarze0 opened this issue Jul 13, 2022 · 8 comments · Fixed by #570

Comments

@SaschaSchwarze0
Copy link

SaschaSchwarze0 commented Jul 13, 2022

The node-start buildpack at the moment looks just for js files as start scripts, https://github.com/paketo-buildpacks/node-start/blob/v0.8.1/node_application_finder.go#L32. If I use ECMAScript modules and therefore import statements and name my file with mjs as extension, then it won't work.

A sample code repository that today cannot be built is https://github.com/SaschaSchwarze0/dump-headers.

Describe the Enhancement

Extend node-start to also look at mjs files.

Possible Solution

This must be extended at those places:

Motivation

Adoption of ECMAScript modules is growing.

@mhdawson
Copy link
Member

mhdawson commented May 2, 2023

I can take a look at this as a follow on to paketo-buildpacks/libnodejs#8

@SaschaSchwarze0 is your suggestion just that it looks for the same files with an mjs extension in addition to the .js ones ?

One question is also how it should affect the ordering. My first take would be same ordering in terms of server, app etc. but look for both .js and .mjs for each before moving to the next. Does that make sense to you?

@SaschaSchwarze0
Copy link
Author

One question is also how it should affect the ordering. My first take would be same ordering in terms of server, app etc. but look for both .js and .mjs for each before moving to the next. Does that make sense to you?

Makes sense, yes. Thank you.

@mhdawson
Copy link
Member

mhdawson commented May 4, 2023

@nodejs/maintainers does what I outlined in #225 (comment) sound good to you? If so I'll submit a PR once some of the existing ones I have open land.

@mhdawson
Copy link
Member

mhdawson commented May 4, 2023

opps wrong mention

@paketo-buildpacks/nodejs-maintainers does what I outlined in #225 (comment) sound good to you? If so I'll submit a PR once some of the existing ones I have open land.

@c0d1ngm0nk3y
Copy link
Contributor

A sample code repository that today cannot be built is https://github.com/SaschaSchwarze0/dump-headers.

But can't you simple add the environment variable BP_LAUNCHPOINT=server.mjs. Works for me.

@SaschaSchwarze0
Copy link
Author

A sample code repository that today cannot be built is https://github.com/SaschaSchwarze0/dump-headers.

But can't you simple add the environment variable BP_LAUNCHPOINT=server.mjs. Works for me.

Yep, this is more about what works ootb without that you must customize something. Is imo hard to explain that you evolve your app to modules, by that use mjs as extension and suddenly the build does not work anymore (without env customization).

@mhdawson
Copy link
Member

mhdawson commented Aug 6, 2024

This dropped off my radar, will try to get back to it.

@mhdawson
Copy link
Member

mhdawson commented Aug 8, 2024

Submitted - paketo-buildpacks/libnodejs#31 as the first step. Once that lands we can add some additional testing into this buildpack to validate the .mjs cases.

@mhdawson mhdawson added semver:minor A change requiring a minor version bump and removed semver:minor A change requiring a minor version bump labels Aug 8, 2024
pacostas pushed a commit to paketo-buildpacks/libnodejs that referenced this issue Aug 9, 2024
mhdawson added a commit to mhdawson/libnodejs that referenced this issue Aug 20, 2024
pacostas pushed a commit to paketo-buildpacks/libnodejs that referenced this issue Aug 22, 2024
mhdawson added a commit to mhdawson/node-start that referenced this issue Aug 22, 2024
Fixes: paketo-buildpacks#225

- add support for mjs and cjs files when looking for applications

Signed-off-by: Michael Dawson <[email protected]>
pacostas pushed a commit to mhdawson/node-start that referenced this issue Sep 2, 2024
Fixes: paketo-buildpacks#225

- add support for mjs and cjs files when looking for applications

Signed-off-by: Michael Dawson <[email protected]>
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 a pull request may close this issue.

3 participants