Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

Added check for log.gauge.isEnabled #60

Closed
wants to merge 1 commit into from
Closed

Added check for log.gauge.isEnabled #60

wants to merge 1 commit into from

Conversation

JoshuaKGoldberg
Copy link

Older versions of gauge do not have isEnabled, and some dependency graphs resolve to it. Fixes #48.

I don't like checking for previous versions of the API but I'm also tired of getting giant error stacks every time I install via yarn. Short of updating the gauge version in everybody who uses it (if only!), there doesn't seem to be any other way to fix 48. 😢

log.progressEnabled = log.gauge.isEnabled()
} else {
// older versions of gauge do not have isEnabled, and some dependency graphs resolve to it
// see https://github.com/npm/npmlog/issues/48
Copy link
Author

Choose a reason for hiding this comment

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

Should there be some kind of warning here?

If not, I can just remove the else and move the comment above the if. Please advise 😊

Copy link
Contributor

Choose a reason for hiding this comment

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

Previous behavior was to explicitly set log.progressEnabled to false, so I would do that here.

@vcfvct
Copy link

vcfvct commented Aug 25, 2018

Would be great if this can be merged. this gauge error is annoying

@jgoux
Copy link

jgoux commented Sep 20, 2018

I have to reinstall npm-check-updates as soon as I make an update with yarn because of this. Could someone merge this please? 🙏

@JoshuaKGoldberg
Copy link
Author

Ping @iarna - I see this repo hasn't had a new commit in a little while, but could we please have a patch version?

@iarna
Copy link
Contributor

iarna commented Sep 20, 2018

Ok, I'm confused… as far as I can tell, the only way for npmlog to be installed with a gauge without isEnabled is for the install to be completely mangled.

This is why… isEnabled was added in gauge in 2.7.0:

v2.7.0

  • New feature: Add new isEnabled method to allow introspection of the gauge's "enabledness" as controlled by .enable() and .disable().

And npmlog added its use of isEnabled in 4.0.1, which ALSO added [email protected]:

49b6d08 We expect the progress bar to be disabled at start (Rebecca Turner on 2016-11-03)
2994674 [email protected] (Rebecca Turner on 2016-11-03)
ec01095 Initialize progressEnabled to gauge.isEnabled() (Rebecca Turner on 2016-11-03)
0197b63 Stop unnecessarily calling showProgress / clearProgress (Rebecca Turner on 2016-11-03)
250b1f3 Fix behavior around pause and the progress bar (Rebecca Turner on 2016-11-03)
a816597 Add changelog for 4.0.1 (Rebecca Turner on 2016-11-16)
c027c27 (tag: v4.0.1) 4.0.1 (Rebecca Turner on 2016-11-16)

Can you share a lock file from a project where this is failing?

I dug in further and see this is caused by a bug in Yarn's handling of bundled dependencies.

Copy link
Contributor

@iarna iarna left a comment

Choose a reason for hiding this comment

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

For me to take this, this should also change the required version range for gauge to include the version that your Yarn bug needs. That will at least making the resulting Yarn tree valid.

log.progressEnabled = log.gauge.isEnabled()
} else {
// older versions of gauge do not have isEnabled, and some dependency graphs resolve to it
// see https://github.com/npm/npmlog/issues/48
Copy link
Contributor

Choose a reason for hiding this comment

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

Previous behavior was to explicitly set log.progressEnabled to false, so I would do that here.

@aeschright
Copy link

@JoshuaKGoldberg Are you still interested in working on this?

@JoshuaKGoldberg
Copy link
Author

Oh hi @aeschright. No, sorry, I didn't notice @iarna's comments back in September but have since lost interest and don't have the time to jump back in. Feel free to take my code and do whatever you wish. 😄

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

Successfully merging this pull request may close these issues.

TypeError: log.gauge.isEnabled is not a function
5 participants