Move all JavaScript projects into single PNPM workspace

[jonkoops]

Moves all of the JavaScript projects into a single PNPM workspace, so that dependencies can be shared. This PR also moves the Node.js and PNPM setup, and installation of dependencies into the pluginManagement section of the root POM so it can be re-used, reducing the boilerplate in child modules.

This PR also:

• Moves the pnpmInheritsProxyConfigFromMaven setting into the root POM and disables it by default.
• Updates Node.js, PNPM and the frontend-maven-plugin to the latest version.
• Cleans up some inconsistent PNPM command usage.

Closes #24571

[jonkoops]

@ASzc @stianst since this moves around quite a lot of build related code, I'd like your opinion on this change.

[ASzc]

Seems like it would work. I don't think it would break the assemblies, since we rely on pnpm pack, which in theory is aware of the new global workspace location.

The run-build execution seems like a candidate for moving into the global config too, pretty sure you can override the configuration where required locally. It would save a few repetitions.

Configuring maven clean plugin to also delete the global workspace dir might be desirable. It would no longer be deleted by default because it's not in a target/ directory

[cypress]

3 flaky tests on run #9791 ↗︎

0

538

48

0

3

Details:

Merge 5476639bad88e54d68a74598c83eaa6460bf75d6 into d4cee15...
Project: Keycloak Admin UI	Commit: 1469a88728 ℹ️
Status: Passed	Duration: 10:26 💡
Started: Nov 14, 2023 10:57 AM	Ended: Nov 14, 2023 11:08 AM

  clients_test.spec.ts • 2 flaky tests • chrome

View
Output

Test		Artifacts
Clients test > Keys tab test > Generate new keys		Test Replay Screenshots
Clients test > Accessibility tests for clients > Check a11y violations on load/ clients list tab		Test Replay Screenshots

  realm_settings_general_tab_test.spec.ts • 1 flaky test • chrome

View
Output

Test		Artifacts
Realm settings general tab tests > Test all general tab switches		Test Replay Screenshots

Review all test suite changes for PR #24537 ↗︎

[jonkoops]

The run-build execution seems like a candidate for moving into the global config too, pretty sure you can override the configuration where required locally. It would save a few repetitions.

Yeah, this is something I'd like to optimize as well. But I am not quite sure how to disable this 'build' step in places where we don't need it. I'll do some research on this.

Configuring maven clean plugin to also delete the global workspace dir might be desirable. It would no longer be deleted by default because it's not in a target/ directory

Which directories are you referring to here? If we're talking about the dist/ directories, those are cleaned automatically by the build scripts on the NPM side already.

[ASzc]

The semi-conventional way to do a global plugin execution config that isn't needed everywhere is to bind the execution in the parent pom to a non-existent phase, then override that execution's phase where you need it to something that exists.

I haven't tried it, but:

<workingDirectory>${maven.multiModuleProjectDirectory}</workingDirectory>

implies to me that there's a global working directory for frontend somewhere in the root directory

[tsaarni]

I've just submitted #24571 couple of days ago which is about failing builds due to pnpm errors. It started happening after #17401 and it can be reproduced by running parallel build

mvn -T4C clean install -DskipTestsuite -DskipExamples -DskipTests

I tried the same with this PR and it is still reproducible. The errors look like following (not all fail at the same time):

[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.14.2:pnpm (pnpm-install) on project keycloak-js-admin-client: Failed to run task: 'pnpm install --prefer-offline --frozen-lockfile --ignore-scripts' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.14.2:pnpm (pnpm-install) on project keycloak-themes: Failed to run task: 'pnpm install --prefer-offline --frozen-lockfile --ignore-scripts' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.14.2:pnpm (pnpm-install) on project keycloak-js-adapter: Failed to run task: 'pnpm install --prefer-offline --frozen-lockfile --ignore-scripts' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.14.2:pnpm (pnpm-install) on project keycloak-js-adapter-jar: Failed to run task: 'pnpm install --prefer-offline --frozen-lockfile --ignore-scripts' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.14.2:pnpm (pnpm-install) on project keycloak-account-ui: Failed to run task: 'pnpm install --prefer-offline --frozen-lockfile --ignore-scripts' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.14.2:pnpm (pnpm-install) on project keycloak-admin-ui: Failed to run task: 'pnpm install --prefer-offline --frozen-lockfile --ignore-scripts' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]

I think the same problem happens when opening Keycloak in vscode with Language Support for Java(TM) by Red Hat:

My previous workflow included executing initial full build with maven and then continuing inside vscode, running IDELauncher class in debugger. This stopped working. Simply opening vscode seems to break the target build with random errors such as:

2023-10-27 13:37:31,544 ERROR [org.keycloak.services.error.KeycloakErrorHandler] (executor-thread-4) Uncaught server error: org.keycloak.theme.FreeMarkerException: Failed to process template index.ftl
...
Caused by: freemarker.template.TemplateNotFoundException: Template not found for name "index.ftl".

and I'm observing the same pnpm-install errors in vscode language server logs. I'm not 100% sure what is going on, but what might be happening is that the language server does parallel compilation, it stops at the pnpm errors and never finishes the build, therefore messing up even the previously executed maven build - one cannot launch Keycloak anymore even via quarkus dev mode from command line while vscode is open. Full non-parallel build with maven on command line can be run to fix it, only to be broken again by the language server immediately afterwards.

[ASzc]

Some kind of locking might work to bypass that, but I don't think the keycloak project has ever claimed to support parallel builds. In fact, I can think of other ways that the POMs have been constructed that would break parallel builds, even outside of the javascript parts

[tsaarni]

It did seem to work before #17401 :( The worst thing is the vscode Java language server problem, where I suspect it causes unfinished build just when opening the project in the editor.

[tsaarni]

FYI I've added some logs & reproduce steps in #24571 (comment)

[tsaarni]

I like this PR, seems good improvement to me 👍

I've spent some time studying the javascript build and I added one suggestion inline, related to the problems I was writing about previously. It is a simple way to avoid all concurrency issues without locking or anything complicated - as well as avoiding unnecessarily executing node install and pnpm install multiple times.

I have also small additional change that I'd like to propose but I'll submit it separately, since they are not related to any lines changed in this PR.

[jonkoops]

Pushed a couple of changes, namely:

• Applied @ASzc's suggestion of also adding the run build step to the root execution. Setting the phase to none (a bogus value) indeed prevents unnecessary execution in places where we don't need to run this step, such as the themes directory.
• The command for the PNPM install is now in-lined, rather than using a property. Since it was only referenced this in a single location anyways it was just adding unnecessary indirection.
• The documentation around upgrading the packages in the node_modules directory has been removed, as we already handle this in an automated fashion with Dependabot.
• The Dependabot configuration has been updated so that it will only target the package file in the root of the project, since this is the root of the workspace it will automatically pick up all other projects in the workspace.

[jonkoops]

I haven't tried it, but:

<workingDirectory>${maven.multiModuleProjectDirectory}</workingDirectory>

implies to me that there's a global working directory for frontend somewhere in the root directory

Correct, all of the things related to the front-end are moved as much as possible into the root of the workspace in this PR. That includes the node directory created by maven-frontend-plugin, which contains the Node.js and PNPM binaries, as well as the packages installed by PNPM.

[jonkoops]

@ASzc can I ask you to take another look at the changes here and let me know if things look good to you?

[jonkoops]

Pushed a change to bump PNPM to the latest version, rebased the PR and squashed the commits.

[ASzc]

LGTM

[jonkoops]

Not sure why, but I now have some failing tests that were previously passing just fine. I'll have another look 🙄

[jonkoops]

I am marking this as ready to review. Now the the the old Account Console has been removed there are no longer weird dependency mismatches preventing this from working. Additionally I've also upgraded the Node.js version to 20, which is is the latest LTS version.

[jonkoops]

@ASzc @tsaarni @agagancarczyk could I ask you for another round of reviews?

[jonkoops]

Pushed a little more cleanup and also updated the frontend-maven-plugin while we're at it.

[agagancarczyk]

LGTM!

[ASzc]

This only very slightly broke the product build, and I've fixed it. Looks good

[jonkoops]

This only very slightly broke the product build, and I've fixed it. Looks good

Good to hear it was an easy fix, what caused the issue?

[ASzc]

I don't know exactly, but the error was that the npm binary was already present when the build setup script when to create it. I just added the force flag to ln and it worked. Possibly something to do with the workspaces being merged by this PR, but I didn't dig into it enough