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

Use update-notifier lib with dynamic import #298

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

danieljung-seon
Copy link

@danieljung-seon danieljung-seon commented May 31, 2023

Summary/ Motivation (TLDR;)

Potential solution for the open #276 🐛

When trying to use the latest prerelease version that has update-notifier@6 installed I get the following error:

/jest-preview/dist/cli/index.js:5
var updateNotifier = require('update-notifier');
                     ^

Error [ERR_REQUIRE_ESM]: require() of ES Module ...

Inspiration comes from the Please read this section's choice num. 2 of the [email protected] release.

  1. If the package is used in an async context, you could use await import(…) from CommonJS instead of require(…).

Fixes

  • Might fix version 0.3.2-alpha.1 that has a problem with update-notifier@6 being used with require. (commonjs context)

Quick test I did:

  • Faked an older version:
Screenshot 2023-05-31 at 9 43 14
  • Created dist files by running pnpm run build
  • Ran node dist/cli/index.js twice and got the following message:
Screenshot 2023-05-31 at 9 43 09

PS:

  • Would be nice to create a new prerelease and another round of testing to see if the change indeed works.
  • Let me know if this is the right track or not, or if there's anything I could change.

@netlify
Copy link

netlify bot commented May 31, 2023

Deploy Preview for jest-preview-library canceled.

Name Link
🔨 Latest commit 24cbe84
🔍 Latest deploy log https://app.netlify.com/sites/jest-preview-library/deploys/6476ff51be96e50008d1015f

@netlify
Copy link

netlify bot commented May 31, 2023

Deploy Preview for jest-preview-chinese canceled.

Name Link
🔨 Latest commit 24cbe84
🔍 Latest deploy log https://app.netlify.com/sites/jest-preview-chinese/deploys/6476ff5142bdc10008a26d10

@danieljung-seon
Copy link
Author

@nvh95 do you think this is worthy?

@snout-o
Copy link

snout-o commented Jan 23, 2024

Any chance we can have this change merged so we can update "update-notifier" to v6?

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 this pull request may close these issues.

3 participants