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: Support ESM apps #1685

Closed

Conversation

AaronDewes
Copy link
Member

@AaronDewes AaronDewes commented May 8, 2022

Fixes #1684.

This would require Node 14, which is a breaking change, but Node 14 is the oldest still supported LTS version, so I think it's not a problem

@AaronDewes
Copy link
Member Author

The tests work locally, I dont know why they fail on actions

@AaronDewes AaronDewes requested a review from gr2m May 8, 2022 16:59
@AaronDewes
Copy link
Member Author

The fail is related to nodejs/node#35889. --experimental-vm-modules is only required in Jest, not when actually using the app. I'm not sure what's the best way to proceed here. Trying to run the different code in the tests defeats the point of the tests

@mattwynne
Copy link

Thanks for looking at this.

I'm no expert on this CJS/ESM stuff, but is it possible to change the tsconfig settings so that you transpile to JavaScript that uses ESM imports instead of the require?

@AaronDewes
Copy link
Member Author

Yes, that is possible, but would require more major refactoring changes to be native ESM.

I can try to do that, but it could break some apps using probot, it'd require a lot of testing.

@wolfy1339 wolfy1339 changed the base branch from master to beta October 29, 2023 02:13
@wolfy1339 wolfy1339 changed the title Support ESM apps feat: Support ESM apps Oct 29, 2023
@wolfy1339 wolfy1339 force-pushed the aarondewes/support-esm-apps-1684 branch from b906ded to db47312 Compare October 29, 2023 02:16
@AaronDewes
Copy link
Member Author

Part of #1821.

@AaronDewes AaronDewes closed this Nov 10, 2023
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.

Support ESM apps
3 participants