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

Fix Commander Program deprecated default program import with named import #39932

Merged
merged 1 commit into from
Apr 1, 2024
Merged

Conversation

aav7fl
Copy link
Contributor

@aav7fl aav7fl commented Mar 29, 2024

Fixes:

The amphtml-validator npm package is currently broken in 1.0.36 if one tries to use the command:

npx amphtml-validator index.html
/workspaces/website/node_modules/amphtml-validator/index.js:378
      .usage(
       ^

TypeError: program.usage is not a function
    at main (/workspaces/website/node_modules/amphtml-validator/index.js:378:8)
    at Object.<anonymous> (/workspaces/website/node_modules/amphtml-validator/cli.js:7:1)
    at Module._compile (node:internal/modules/cjs/loader:1369:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1427:10)
    at Module.load (node:internal/modules/cjs/loader:1206:32)
    at Module._load (node:internal/modules/cjs/loader:1022:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12)
    at node:internal/main/run_main_module:28:49

This is because the most recent release of amphtml-validator introduces a breaking change with the v12.0.0 Commander dependency.

Per migration docs of Commander v12, use the named import instead of the deprecated default import.

https://github.com/tj/commander.js/tree/v12.0.0

https://github.com/tj/commander.js/blob/v12.0.0/docs/deprecated.md#default-import-of-global-command-object

Making this change outlined in their migration note (and locally), it fixes my amphtml-validator execution of files.

Emojis for categorizing pull requests (copy-paste emoji into description):

🐛 Bug fix

@CLAassistant
Copy link

CLAassistant commented Mar 29, 2024

CLA assistant check
All committers have signed the CLA.

@amp-owners-bot amp-owners-bot bot requested a review from amaltas March 29, 2024 13:20
Copy link

amp-owners-bot bot commented Mar 29, 2024

Hey @ampproject/wg-caching! These files were changed:

validator/js/nodejs/index.js

Copy link
Contributor

@powerivq powerivq left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution

@powerivq powerivq requested review from erwinmombay and removed request for amaltas April 1, 2024 03:02
@powerivq
Copy link
Contributor

powerivq commented Apr 1, 2024

@erwinmombay for approval

@erwinmombay erwinmombay merged commit f2bc788 into ampproject:main Apr 1, 2024
51 checks passed
@aav7fl aav7fl deleted the patch-1 branch April 1, 2024 19:37
eos87 added a commit to liveblog/liveblog that referenced this pull request Apr 2, 2024
The commander dependency on `amp-validator` was bumped to the version 12.0.0 which is not compatible with out current version of Node.js

ampproject/amphtml#39932

LBSD-2733
eos87 added a commit to liveblog/liveblog that referenced this pull request Apr 25, 2024
The commander dependency on `amp-validator` was bumped to the version 12.0.0 which is not compatible with out current version of Node.js

ampproject/amphtml#39932

LBSD-2733
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.

5 participants