Problem/Motivation

Update cspell for Drupal 12.

Steps to reproduce

Proposed resolution

Remaining tasks

postponed #3620803: Update minimum Node version to 24. this needs NodeJs> 22.18.0

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#6 3621993-nr-bot__8_3sjf6.txt27.41 KBneeds-review-queue-bot

Issue fork drupal-3621993

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

quietone created an issue. See original summary.

quietone opened merge request !17056

quietone’s picture

quietone commented
Issue summary: View changes
Status: Active » Postponed

Reported error:
Unsupported NodeJS version (20.20.2); >=22.18.0 is required

quietone’s picture

quietone commented
Parent issue: #3621277: Update JavaScript dependencies for 12.0.0-beta1 » #3449808: [12.x] [meta] Requirements for tagging 12.0.0-beta1
quietone’s picture

quietone commented
Status: Postponed » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3621993-nr-bot__8_3sjf6.txt27.41 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

quietone’s picture

quietone commented
Status: Needs work » Needs review
Issue tags: +no-needs-review-bot

That was fast, 20 minutes to say the MR doesn't apply. Poppycock.

The bot reports include this, so it is using an old version of node. 

4.18.0

Running spellcheck on *all* files.
Unsupported NodeJS version (20.20.2); >=22.18.0 is required

CSpell: failed
godotislate’s picture

godotislate
he/him
commented
Status: Needs review » Reviewed & tested by the community

Changes are straightforward. lgtm

Is this going to 11.x? If so, we might need to check that the dictionary changes are fine there too before committing.

mstrelan’s picture

mstrelan commented

I was surprised to see dictionary changes here, but quietone explained to me that we run yarn spellcheck:make-dict and spellcheck:make-drupal-dict whenever we do a cspell upgrade.

I wanted to know more and had an LLM assist with some automation. It found that 27 words would have been removed before the upgrade, and a further 17 words are removed due to the upgrade. I also learned about the cspell trace command, that investigates which dictionary a word belongs to, including internal cspell dictionaries.

I was also surprised to see pcre's added to the dictionary. I believe it can be removed, but I think we might need to update the tooling to prevent it coming back in. Cspell already allows pcre's when we have pcre in the dictionary, but when we delete the dictionary it reports both. There are a few other instances of this, like overrider's. Perhaps we can have a follow up to remove these?

I suspect this is not being backported to 11.x since the cspell update requires Node 22.

EDIT: Follow up #3623854: Strip possessives from cspell dictionaries

quietone’s picture

quietone commented
Issue tags: +12.0.0 beta blocker
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Conflicts in the yarn.lock file... can be set back to rtbc once fixed.

quietone’s picture

quietone commented
Status: Needs work » Reviewed & tested by the community

Rebased and tests passed

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 24d352f31a9 to main. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

xjm’s picture

xjm
she/her
Primary language English
commented

Is there any chance this somehow broke which files are ignored when cspell is run locally? When I just tried to commit a patch on main (fresh node_modules and all that) it spellchecked MAINTAINERS.txt and (obviously) failed. But I don't see any CR here documenting a different behavior, and the MR was passing.