Problem/Motivation

In order to catch unintentional regressions such as enablePlaceholder() in CKEditor 5 39 (see #3377562-14: Update CKEditor 5 to 39.0.0) long ahead of time, let's have an issue that is forever chasing the nightly tag of CKEditor 5.

Automated CKEditor 5 nightly changelog: https://github.com/ckeditor/ckeditor5/issues/14724

To update the MR once you have it locally:

git checkout 3379104-cke5-nightly
git pull
cd core && yarn upgrade --pattern ckeditor5 && yarn build && yarn build:ckeditor5-types
cd .. && git add core/assets/vendor/ckeditor5/ && git commit -nam "$(date)"
git push --set-upstream drupal-3379104 HEAD

⚠️ Whenever the version of CKEditor 5 in the upstream branch (11.x) has changed, you need to remove all the "date" commits and revert back to Pin all CKEditor 5 packages to the `nightly` tag. commit, pull & rebase, and force push:

git checkout 3379104-cke5-nightly
git pull
git reset --hard $(git log 11.x..3379104-cke5-nightly --format=%h | tail -2 | head -1)
git fetch origin
git rebase origin/11.x --strategy-option theirs
# Now do the usual steps, but skip the checkout + pull, and do a force push:
cd core && yarn install && yarn upgrade --pattern ckeditor5 && yarn build && yarn build:ckeditor5-types
cd .. && git add core/assets/vendor/ckeditor5/ && git commit -nam "$(date)"
git push -f --set-upstream drupal-3379104 HEAD

Steps to reproduce

N/A

Proposed resolution

N/A

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3379104

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

Wim Leers created an issue. See original summary.

Wim Leers opened merge request !4536

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Priority: Normal » Major
Issue summary: View changes
Issue tags: -JavaScript +JavaScript

Done, and provided instructions to update this MR 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: -JavaScript +JavaScript

Alright, that's a success! 😄 https://www.drupal.org/pift-ci-job/2732147 failed on the exact problem that was discovered when creating the MR for the latest CKEditor 5 upgrade (#3377562-14: Update CKEditor 5 to 39.0.0), this clearly would have surfaced it a long time ago 👍

I'll update this periodically. In my absence, anybody else can do the same using the commands in the issue summary 😊

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

Linking the automated CKEditor 5 nightly changelog: https://github.com/ckeditor/ckeditor5/issues/14724 🥳

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

Refined commands as I did the first update 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

That MR didn't apply anymore. The reason being: the upstream had changed from 38.1.0 to 39.0.0 in the mean time.

So updating the issue summary for how to deal with that.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#3380637: Update CKEditor 5 to 39.0.1 landed, redid the MR again 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

Turns out yarn install does not do anything anymore once you're already on a nightly 🙄

Turns out we need yarn upgrade --pattern ckeditor5 … probably. Seems like nobody really knows how to make this package manager do what you want: just what you want and nothing more.

Tweaked the issue summary once more…

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Worked like a charm this time! 🚀

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » wim leers
Related issues: +#3393557: [upstream] Update CKEditor 5 to 40.0.0

Now that #3393557: [upstream] Update CKEditor 5 to 40.0.0 is in, tests are green again, and I should get this going again.

catch’s picture

catch
he/him
Primary language English
commented

I wonder if we could make a special purpose pipeline that runs on a weekly schedule and not alongside the other pipelines, that updates to ckeditor5 nightly and runs just the ckeditor5 tests. It could do a similar thing to what the new performance job that just landed does in terms of exclusive scheduling.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [meta] Test against CKEditor 5 nightly » Add a "CKEditor 5 nightly" GitLab CI job
Category: Plan » Task
Issue tags: +GitLab
Related issues: +#3391689: Add a performance tests job to gitlab and send data to OpenTelemetry

@catch I'd love to do that! As you can see in the issue summary, I've already scripted it anyway. I'll do that! 🤓 When I started this, core wasn't yet on GitLab CI and I knew far less about it. Should be easy now 😄

Wim Leers opened merge request !5325

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review

First step is made: https://git.drupalcode.org/project/drupal/-/merge_requests/5325/diffs?co... :)

Also: yarn is weird.

justafish made their first commit to this issue’s fork.

justafish opened merge request !5331

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » justafish
Status: Needs review » Needs work

I got very excited there, but it's not quite working yet 😄

Getting closer though! 🤞

The CKEditor Nightly + PHP 8.2 MySQL 8 child pipeline is not yet copying the results of 🌌 CKEditor Nightly into every job in that child pipeline. Without that, this won't work correctly.

At a minimum, we'd need the yarn nightly update results to be applied when running FunctionalJavaScript and Nightwatch tests.

catch’s picture

catch
he/him
Primary language English
commented

At a minimum, we'd need the yarn nightly update results to be applied when running FunctionalJavaScript and Nightwatch tests.

Two options I think:

1. If the variable from the schedule is set, then make the child jobs require the artifacts from the nightly job.

2. Don't try to run the full test suite and instead manually call phpunit with @group ckeditor5 from the nightly job itself after building.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#19.1: I don't think a conditional needs is supported by GitLab CI? 🤔

catch’s picture

catch
he/him
Primary language English
commented

@Wim Leers no, but:

.with-composer-and-yarn: &with-composer-and-yarn
  needs:
    - pipeline: $PARENT_PIPELINE_ID
      job: '📦️ Composer'
    - pipeline: $PARENT_PIPELINE_ID
      job: '📦️ Yarn'

Yarn could be a variable that is conditionally assigned to either the standard job or the ckeditor5 upgrade job I would think.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

AFAICT that's not supported per https://docs.gitlab.com/ee/ci/yaml/#needs.

https://stackoverflow.com/questions/70003527/gitlab-ci-does-not-support-... seems to confirm this.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

🤔 Maybe the simpler solution then is to have conditional logic in the 📦️ Yarn job, based on some top-level variable? That'd achieve the same as #21.

catch’s picture

catch
he/him
Primary language English
commented

Oh that is unfortunate.

I think we would essentially have to make the yarn job responsible for all the ckeditor nightly logic, but that does seem feasible.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

No time to work on the GitLab CI job today. Doing the manual process takes only a few minutes, so did that. 40.1.0 is shipping with Drupal 10.2 soon (thanks to #3400791: Update CKEditor 5 to 40.1.0), so about time to start testing again against the current nightly.

Wim Leers changed the visibility of the branch 3379104-cke5-nightly-ci to hidden.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

The current CKE5 nightly caused a regression. Looks like https://github.com/ckeditor/ckeditor5/commit/0647ba65cd09575937a9ba8e60b... caused this, because only ImageUrlTest is failing: https://git.drupalcode.org/issue/drupal-3379104/-/pipelines/60411/test_r....

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#3406732: Update CKEditor 5 to 40.2.0 was merged, and shipped in Drupal 10.2.0 6 days ago. Time for testing the new nightly! https://git.drupalcode.org/project/drupal/-/merge_requests/4536/diffs?co...

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

~1 month after #30 (which passed tests 👍): testing against today's nightly: https://git.drupalcode.org/project/drupal/-/merge_requests/4536/diffs?co...

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3401988: Spell-checking job fails with "Argument list too long" when too many files are changed

Failed on spell check due to the 11.x branch in this issue fork being so far behind. See #3401988: Spell-checking job fails with "Argument list too long" when too many files are changed. Manually pushed a version of 11.x that matches the starting point of the current MR.

fjgarlin made their first commit to this issue’s fork.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Pushed newest nightly now that #3401988: Spell-checking job fails with "Argument list too long" when too many files are changed is fixed 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I got it wrong — I needed to rebase on top of the latest 11.x. Done now 👍

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: justafish » wim leers

That's failing with

Uncaught TypeError: g.Model is not a constructor
    at drupalMedia.js?s9md41:1:33420
    at Array.forEach (<anonymous>)
    at ie.getDropdownListItemDefinitions (drupalMedia.js?s9md41:1:33378)
    at Object.callback (drupalMedia.js?s9md41:1:34300)
    at Co.create (ckeditor5-dll.js?v=0.0.0-nightly-20240129.0:5:551016)
    at ckeditor5-dll.js?v=0.0.0-nightly-20240129.0:5:514855
    at Array.map (<anonymous>)
    at et._buildItemsFromConfig (ckeditor5-dll.js?v=0.0.0-nightly-20240129.0:5:514763)
    at et.fillFromConfig (ckeditor5-dll.js?v=0.0.0-nightly-20240129.0:5:514621)
    at $._showToolbar (ckeditor5-dll.js?v=0.0.0-nightly-20240129.0:5:682145)

😬

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » Unassigned

I fixed #36 in #3424644: Update CKEditor 5 to 41.2.0. That just landed in core! 😄

Time to start chasing the nightly again. Just pushed that 👍

New releases expected in: April and May, June maybe.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

The sole test failure seems unrelated and possibly a recent core regression:


Drupal\Tests\Composer\Plugin\Scaffold\Functional\ManageGitIgnoreTest::testUnmanagedGitIgnoreWhenGitNotAvailable
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Scaffolding files for fixtures/drupal-assets-fixture:\n
+'Composer could not detect the root package (fixtures/drupal-composer-drupal-project) version, defaulting to '1.0.0'. See https://getcomposer.org/root-version\n
+Scaffolding files for fixtures/drupal-assets-fixture:\n
   - Copy [web-root]/.csslintrc from assets/.csslintrc\n
   - Copy [web-root]/.editorconfig from assets/.editorconfig\n
   - Copy [web-root]/.eslintignore from assets/.eslintignore\n

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.