Skip to content

building docker image is failing since 5.8.0 #1798

Open
@bourgeoa

Description

@bourgeoa

@angelo
building docker image is failing since 5.8.0
the build has been updated to node 20 alpine and related test
Could it be the use of node --experimental-require-module ?

Activity

changed the title [-]docker image is failing[/-] [+]building docker image is failing[/+] on Nov 7, 2024
changed the title [-]building docker image is failing[/-] [+]building docker image is failing since 5.8.0[/+] on Nov 7, 2024
changed the title [-]building docker image is failing[/-] [+]building docker image is failing since 5.8.0[/+] on Nov 7, 2024
melvincarvalho

melvincarvalho commented on Apr 9, 2025

@melvincarvalho
Contributor

Could it be the use of node --experimental-require-module

Yes—we shouldn’t be running with this experimental flag. It was deprecated in Node 20 and removed entirely in Node 21. Using experimental flags makes the Docker build fragile and incompatible with modern Node versions. Docker is important to NSS, and this breaks it.

This feels like a breaking change that was merged too early. I’d strongly favour reverting it, or at the very least, moving it to a separate branch until it’s properly supported and integrated.

We’re already on node:lts (Node 20.x), so it’s time to drop or isolate this.

melvincarvalho

melvincarvalho commented on Apr 9, 2025

@melvincarvalho
Contributor

I suggest reverting to 5.7 and making a new experimental branch for experimental features that can be checked out and run on an instance with instructions on how to do that.

csarven

csarven commented on Apr 11, 2025

@csarven
Member

Let's play logical fallacies again.

Straw man: The codebase already uses require() consistently so this isn't a new practice or introduced in v5.8. And there is no proof that the issue is entirely caused by the use of --experimental-require-module. The actual issue with Docker may stem from Node 20's deprecation/removal behavior, not any new code.

False cause: If the Docker breakage stems from Node 20 removing support for that flag, the root issue is environmental, not a code regression in 5.8.

False dilemma: revert everything and isolate in a separate branch are extreme solutions. There are other options, ... patch docker to work w/o deprecated flags... migrate the codebase to use ESM properly... add logic for for dev/prod builds. So, no need to prematurely ignore middle-ground solutions.

Appeal to consequences: Is there an evaluation on 5.8 introducing the break? Docker configs can be updates to modern Node or whatever, or other long term solutions

Overgeneralization: assuming "experimental" equals to inherently unstable or unacceptable, regardless of pragmatic use.


But look, docker is not the only way to run NSS. The codebase hasn't fundamentally changed in its module style.

If there's still concern about 5.8 or suggestions for improvement, feel free to kick off a PR.

bourgeoa

bourgeoa commented on Apr 11, 2025

@bourgeoa
MemberAuthor

node --experimental-require-module may have been the issue for docker.
It shall not be anymore an issue because require-module has been backported in node 20.19.0 and node 22.14.0

Branch fix/issue#1798 has been created to remove the need for experimental-require-module parameter.
Locally tests passes without any issue.
But CI is failing see #1817

If you have any hint I need help.
For now we cannot make any patch to NSS with CI

CxRes

CxRes commented on Apr 11, 2025

@CxRes
Collaborator

--experimental-require-module was introduced in Node 22.0.0 and was later back ported to Node 20.17.0, the Active LTS branch at the time. The flag (not the feature) was dropped in Node 23.0.0 because require ESM has reached RC status. The "experimental" warning has been turned off by default as of 23.5.0.

https://nodejs.org/api/modules.html#loading-ecmascript-modules-using-require

All of this information is available in the NodeJS documentation. The claims made regarding the flag contradict the facts and represents a failure to understand the NodeJS development cycle.

melvincarvalho

melvincarvalho commented on Apr 14, 2025

@melvincarvalho
Contributor

The sarcastic and dismissive tone here — e.g. “let’s play logical fallacies again” and “failure to understand” — is not okay. It crosses into personal attack and violates the expectations of respectful collaboration set out in the W3C Code of Conduct. Let’s please keep this space professional and welcoming to all contributors.

On the technical issue:

  • The Docker image has been broken since 5.8.0 — that’s a regression in real-world usage.
  • It was caused by reliance on a deprecated, experimental flag removed in Node 21+.
  • Whether or not it’s technically backported doesn’t change the fact that it broke builds.

IMHO, we should revert to 5.7.x as the stable release, and treat anything involving unstable flags as experimental until it’s proven solid. Happy to help with that if there’s agreement.

michielbdejong

michielbdejong commented on Apr 14, 2025

@michielbdejong
Member

The sarcastic and dismissive tone here — e.g. “let’s play logical fallacies again” and “failure to understand” — is not okay.

I totally agree, it's maybe not the only reason that we are, as a group, struggling to make peaceful progress in the Solid project, but sarcasm (especially in written comments) does not seem to be a useful tool for us, and it's probably doing more harm than good to the Solid project at this point.

@csarven from now on, please help to keep sarcasm and bitterness out of written interactions if you can?

csarven

csarven commented on Apr 14, 2025

@csarven
Member

I am bowing out of this thread after this comment.


@michielbdejong , if you're genuinely keen on putting effort to making "peaceful progress", I'm so with you, but then I need to request that you ask yourself why you're rushing here attempting to tone police, focusing only on my comments and not others', and why you are not making any remarks on logical fallacies as pointed out, refraining from requesting citations on unsubstantiated claims, or calling out argumentum ad nauseam. Let me know when you'd like to have that rational discussion in good faith.


As for any remaining technical discussions on this issue, there has not been a strong argument put forward to revert anything. If anything, reverting is likely to cause more concerns - after all, 5.8 is out there already, which includes other fixes and features - than making efforts towards improving whatever needs to be done. Reverting is out of the question as I see it (or in any open source project in a similar situation). If anyone cares enough, and willing to put their time, they can focus on putting their energy on fixing or improving the broken bits. Alternatively, they can consider refraining from repeating their request (argumentum ad nauseam) - we understood it the first time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

      Participants

      @melvincarvalho@michielbdejong@csarven@bourgeoa@CxRes

      Issue actions

        building docker image is failing since 5.8.0 · Issue #1798 · nodeSolidServer/node-solid-server