Problem/Motivation

DrupalCI is currently using Node.js 8.16.0 which is EOL since the end of 2019: https://nodejs.org/en/about/releases/. Drupal core would like to drop support for Node.js 8: #3107918: Require Node.js 12.

Proposed resolution

Update to a more recent version of Node.js.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 3107927-13.patch17.89 KBlauriii
#10 output.txt23.84 KBlauriii
#9 nightwatch_node_12.15.0.txt3.44 KBbnjmnm
#9 nightwatch_node_8.16.0.txt3.44 KBbnjmnm
#6 3107927-6.patch936 byteslauriii

Comments

lauriii created an issue. See original summary.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Have we updated DrupalCI Node.js version before? I'm wondering if this will cause issues with contrib projects. Should we instead allow projects to configure which major version of Node.js they would like to use?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Related issues: +#3107918: Require Node.js 12
xjm’s picture

xjm
she/her
Primary language English
commented
Parent issue: » #3009213: [META] Update / reconsider PHP dependencies for Drupal 9
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Project: DrupalCI: Drupal.org Testing Infrastructure » DrupalCI: Environments
Component: Code » PHP Containers
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Active » Needs review
StatusFileSize
new 3107927-6.patch936 bytes

This updates to Node.js 12.x

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue tags: +Drupal 9.0.0-beta1 requirement

Tagging as a requirement for Drupal 9.0-beta1.

hestenet’s picture

hestenet
He/Him
Location Portland, OR 🇺🇸
commented

Thanks very much @laurii! There's one more thing we'd like to test before merging in and deploying - we want to make sure that Nightwatch testing still works properly with this change.

Is that something you can test locally? (No huge rush, we're currently knocking out some unrelated alpha/beta blocker stuff)

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
StatusFileSize
new nightwatch_node_8.16.0.txt3.44 KB
new nightwatch_node_12.15.0.txt3.44 KB

Nightwatch appears to work fine. Attached are the results running 8.16 and 12.15

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new output.txt23.84 KB

Here's a console output of the test runner running Nighwatch tests with Node.js 12. I added step to the test runner which outputs the Node.js version near the end to confirm that the Node.js version is indeed updated. I also tried to run functional JavaScript tests but it seems like I'm getting lots of random failures both before and after this change so I wouldn't be able to provide a consistent output from there.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Update Node.js to a more recent version » Update Node.js to a more recent version on DrupalCI
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +beta target
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3107927-13.patch17.89 KB

Here's a patch that updates to Node 12, but also installs nvm so that the Node version can be configured in the test runner to be one of the pre-installed Node.js versions.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented 
+++ b/dockerfile_templates/base.m4
@@ -6,6 +6,9 @@ FROM debian:jessie
+# Silence any console output from mesg command in .profile.
+RUN sed -i "s/mesg n/mesg n \&\> \/dev\/null/g" ~/.profile
+

What is this, and why is it necessary?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

This is needed so that we can use bash -l on #3124107: Allow selecting Node.js version with nvm. The problem is that mesg n command outputs stdin: is not a tty because when we run commands with bash -c tty hasn't been allocated.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Status: Needs review » Fixed

We did some research and some testing, and determined that there are likely very, very few projects that would be impacted by a wholesale upgrade from 8 -> 12.

We tested this in the dev container environment, and everything appears to be running fine for core testing, so this has now been deployed to production, and drupalci now uses node12 when it does its yarn install step (and also nightwatch tests now rely on node 12)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.