jsdoc: remove deprecated rule jsdoc/newline-after-description

[legobeat]

Will be causing error starting with eslint-plugin-jsdoc@43

gajus/eslint-plugin-jsdoc#1031

[Mrtenz]

From the plugin release:

1. Removes jsdoc/newline-after-description rule in favor of jsdoc/tag-lines with option startLines: 0 for "never" and startLines: 1 for "always". Defaults now to startLines: 0

Should we add jsdoc/tag-lines?

[legobeat]

From the plugin release:

1. Removes jsdoc/newline-after-description rule in favor of jsdoc/tag-lines with option startLines: 0 for "never" and startLines: 1 for "always". Defaults now to startLines: 0

Should we add jsdoc/tag-lines?

Doing so would mean this rule would not be usable in Node.js 14. I don't see the rush. It seems this change coincides with switching from supporting node 14 and 20 for the plugin, so omitting both seems like the only way to support 14~20 here.

[Mrtenz]

From the plugin release:

1. Removes jsdoc/newline-after-description rule in favor of jsdoc/tag-lines with option startLines: 0 for "never" and startLines: 1 for "always". Defaults now to startLines: 0

Should we add jsdoc/tag-lines?

Doing so would mean this rule would not be usable in Node.js 14. I don't see the rush.

Well, I don't think we want JSDoc comments without a line between the description and params.

And since Node.js 14 is EOL, I don't see why we couldn't update this rule?

[legobeat]

From the plugin release:

1. Removes jsdoc/newline-after-description rule in favor of jsdoc/tag-lines with option startLines: 0 for "never" and startLines: 1 for "always". Defaults now to startLines: 0

Should we add jsdoc/tag-lines?

Doing so would mean this rule would not be usable in Node.js 14. I don't see the rush.

Well, I don't think we want JSDoc comments without a line between the description and params.

And since Node.js 14 is EOL, I don't see why we couldn't update this rule?

I'm considering the nature of this package. It's being used in intermediary libraries and CI currently on 14 and on their way up. That eslint itself is still on 12 is likely not an oversight but a conscious effort, if I'd guess.

My reasoning is: Keeping the range open makes it easier for dependents to migrate so (perhaps counter-intuitively) removing support for older runtime versions can slow down deprecations and updates by users and using libraries.

[mcmire]

Is there a way that we can add this conditionally? Detect the version of eslint-plugin-jsdoc being used, and if it's >= 43 then use jsdoc/tag-lines, otherwise stick with jsdoc/newline-after-description?

[Gudahtt]

I don't think we need to worry about compatibility with packages that violate the peerDependency requirements, which are currently:

"eslint-plugin-jsdoc": "^39.6.2",

Agreed with @Mrtenz that it seems better to keep this rule until we can replace it. Unless we decide we don't want it anymore at least. I don't really see the case for dropping this rule so early that we can't yet use its replacement.

[legobeat]

FWIW, trigger for this making change: https://github.com/MetaMask/etherscan-link/actions/runs/4978661619/jobs/8909250375?pr=70

Adding 'jsdoc/newline-after-description': 'off' to the eslintrc does not help. It's less about violating peerDependencies and more about being able to have a single checkout run without errors across engine versions.

Got some pointers on how to make that CI pass on node.js 20 without this either this change having to remove the dependency alltogether (throwing out baby with bathwater imo) or forcing yarn to ignore engines completely?

As I foresee the same cropping up again in potentially several repos, this could save a lot of future time.

[legobeat]

Just to follow up on the last comment to indicate the subtlety here, in case it was not clear before:

Consider for example https://github.com/MetaMask/etherscan-link/blob/c97467095c71e987710d1a61b1b91359d4e4ba16/package.json

factors:

• published library
• package manager yarn
• supports node.js 14
• devDependency @metamask/eslint-config-jsdoc (for the case of this example, the fact that it's on an older version is irrelevant)

current issue:

• yarn install on Node.js 20 will fail with eslint-plugin-jsdoc@39.9.1: The engine "node" is incompatible with this module. Expected version "^14 || ^16 || ^17 || ^18 || ^19". Got "20.1.0"

possible solutions without patching eslint-config

1. require --ignore-engines for yarn operations for the repo
   • easy but should only be a last resort imo
2. ignore peerDependency version of eslint-plugin-jsdoc and use a version indicating support for node 20
   • eslint error Definition for rule 'jsdoc/newline-after-description' was not found jsdoc/newline-after-description
   • adding 'jsdoc/newline-after-description': 'off' to eslintrc does not solve this
   • Note: which plugin versions are available is narrow¹. The first version indicating support for node 20 has it removed already.
3. remove this devDependency
   • 👶 with 🛁 💧
4. disable linting on recent node versions
   • it's a hassle and pushing the problem forward
5. disable ci on recent node versions
   • carries risk

If there is some way from the using package to override and disable the rule that I'm just overlooking, that would resolve this one, though, and make the below point less serious.

future releases

I assume a future release of @metamask/eslint-config-jsdoc is going to move to a future plugin version, and with that drop this rule and replace it with the new jsdoc/tag-lines. This will make the dev environment not work on Node 14 anymore, most likely resulting in breaking releases (though not strictly required it's the most straightforward and seems to be the norm).

Unless we merge this change (or someone makes some magic happen), this means there would be no single setup possible of the linting framework that could run without hacks on Node.js 14~20 and moving between versions would require disabling or changing them.

Being able to upgrade components independently avoids scenarios such as this and this. With more strategic release planning we can make this experience a lot less time-consuming I think.

I'd be the first to agree that moving towards more recent runtime versions is overall desirable but IMO we want to consider this package being used in ecosystem packages where different factors are at play

If the above was a frustrating read: Unless this is addressed, we'll be pushing users of this package down the rabbit hole of figuring out if and how to address this. Lifting the rule and making bridge releases available on npm could save many hours of future human time, I suspect.

---

Exhibit A

As can be seen, viable versions without disabling engines-check is restricted to 43.0.4~43.0.7.

eslint-plugin-jsdoc	engines.node
39.2.0	^14 || ^16 || ^17
39.2.4	^14 || ^16 || ^17
39.2.5	^14 || ^16 || ^17 || ^18
39.3.0	^14 || ^16 || ^17 || ^18
39.3.5	^14 || ^16 || ^17 || ^18
39.3.6	^14 || ^16 || ^17 || ^18
40.0.0	^14 || ^16 || ^17 || ^18 || ^19
40.3.0	^14 || ^16 || ^17 || ^18 || ^19
41.0.0	^14 || ^16 || ^17 || ^18 || ^19
42.0.0	^14 || ^16 || ^17 || ^18 || ^19
43.0.0	^14 || ^16 || ^17 || ^18 || ^19
43.0.1	^14 || ^16 || ^17 || ^18 || ^19
43.0.2	^14 || ^16 || ^17 || ^18 || ^19
43.0.3	^14 || ^16 || ^17 || ^18 || ^19
43.0.4	^14 || ^16 - ^20
43.0.5	^14 || ^16 || ^17 || ^18 || ^19 || ^20
43.0.6	^14 || ^16 || ^17 || ^18 || ^19 || ^20
43.0.7	^14 || ^16 || ^17 || ^18 || ^19 || ^20
43.0.8	>=16
43.0.9	>=16

Footnotes

1. Compressed summary of plugin version and engines.node field ☝️ ↩

[Gudahtt]

Should we add jsdoc/tag-lines?

Doing so would mean this rule would not be usable in Node.js 14

This rule appears to be present in 43.0.7 🤔 https://github.com/gajus/eslint-plugin-jsdoc/blob/v43.0.7/src/rules/tagLines.js

[legobeat]

Should we add jsdoc/tag-lines?

Doing so would mean this rule would not be usable in Node.js 14

This rule appears to be present in 43.0.7 thinking https://github.com/gajus/eslint-plugin-jsdoc/blob/v43.0.7/src/rules/tagLines.js

Somehow I one-off-errored that one in my head every time. Good catch, look like there's a path forward in replacing jsdoc/newline-after-description with jsdoc/tag-lines and pinning at 43.0.7 for next update?

[legobeat]

Added ^43.0.7 to peerDependencies range of sibling packages without actually upgrading here, lmk if that does not make sense #288

Then sorting out the rule switch can be sorted out after. Putting this one in draft until ts 5 compat is solved. Not strictly blocking but makes sense to me to sort that out first.

[mcmire]

Do we still want to do this now that #288 has been merged?

[legobeat]

Do we still want to do this now that #288 has been merged?

It will have to be disabled for next major (this is the reason '^43' support was dropped from #288).

There is no version that supports both jsdoc/newline-after-description and jsdoc/tag-lines, though 43 does maintain nodejs 14 support after the rule change. I'm still thinking dropping first makes sense, and then introducing jsdoc/tag-lines at a later point, rather than doing it in lock-step.

[Gudahtt]

Makes sense. As long as we replace it soon, I'm OK with removing it.

Migrating between these rules in lockstep would force us into a very narrow range for that peer dependency, which would be a bit awkward to deal with.

[Gudahtt]

LGTM!