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

Update blocked ESM only packages (execa, process-exists) #3856

Merged
merged 6 commits into from
May 11, 2023

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented May 10, 2023

For a while, I've wanted to update execa and process-exists but have been unable to because they are ESM only packages. Importing these modules is now possible (after discovering "moduleResolution": "node16" by chance). This PR upgrade the aforementioned packages and integrates the way we now need to import them with the rest of code base.

Please LMK if you have any questions 🙏🏻.


let execa: EsmExeca
let doesProcessExist: EsmProcessExists
const importEsmModules = async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] The reason that we cannot simply await import('execa') before we call it is that whenever we create a process the function has to be sync. If a Promise<Process> is returned then we cannot attach anything to it. We can only await the stdout.

Copy link
Member Author

Choose a reason for hiding this comment

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

this is still relevant

@mattseddon mattseddon force-pushed the use-esm-modules branch 2 times, most recently from fd91f06 to 1fdc13f Compare May 10, 2023 04:07
@mattseddon mattseddon marked this pull request as ready for review May 11, 2023 05:12
@mattseddon mattseddon requested review from sroy3 and julieg18 as code owners May 11, 2023 05:12
@mattseddon mattseddon marked this pull request as draft May 11, 2023 06:05
@mattseddon mattseddon marked this pull request as ready for review May 11, 2023 06:10
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

Great work!

@mattseddon mattseddon enabled auto-merge (squash) May 11, 2023 19:45
@codeclimate
Copy link

codeclimate bot commented May 11, 2023

Code Climate has analyzed commit 85a9c91 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 94.9%.

View more on Code Climate.

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

Successfully merging this pull request may close these issues.

3 participants