ddev-intellij-plugin icon indicating copy to clipboard operation
ddev-intellij-plugin copied to clipboard

Enabling "Automatic ESLint Configuration" results in unhealthy projects

Open mandrasch opened this issue 9 months ago • 19 comments

Is there an existing issue for this?

  • [x] I have searched the existing issues

Are you sure that this bug is related to this DDEV Integration Plugin?

  • [x] I am sure

Enter your error report ID (If available)

No response

Describe the bug

Enabling "Automatic ESLint Configuration" in PhpStorm Settings and using it in package.json will crash DDEV projects - as soon as you edit a javascript file.

@rfay asked me to report this (Discord)

As far as I understood was this also identified in the past and the reason was that PhpStorm does not use the current DDEV web container to execute commands, but I'll launch new ones instead. This was discussed in https://github.com/ddev/ddev/issues/6125

The challenge for "developer experience" is that we tell DDEV users to just install the add-on (https://ddev.readthedocs.io/en/stable/users/install/phpstorm/#ddev-integration-plugin), they'll assume everything is setup correctly and than their projects crash when they edit JS files and activate ESLint.

Maybe a mitigation strategy like "We detected you enabled ESLint, this won't work" could be used to at least give a hint?

As far as I know this can only be solved currently by switching the NodeJS interpreter to the local node (but then you won't use the configured NodeJS version of .ddev/config.yaml obviously). Please correct me if I'm wrong on this.

Image

Steps to reproduce

  1. Install PhpStorm (Apple Silicon), PhpStorm 2024.3.2.1
  2. Install DDEV addon
  3. Set up a fresh plain PHP project via ddev config and ddev start
  4. Add package.json and ESLint
# create package.json
ddev npm init -y

ddev npm init @eslint/config@latest

@eslint/create-config: v1.4.0

✔ How would you like to use ESLint? · problems
✔ What type of modules does your project use? · esm
✔ Which framework does your project use? · none
✔ Does your project use TypeScript? · typescript
✔ Where does your code run? · browser
The config that you've selected requires the following dependencies:

eslint, globals, @eslint/js, typescript-eslint
? Would you like to install them now? ‣ Yes
✔ Which package manager do you want to use? · npm
  1. Enable "Automatic ESLint Configuration" in PhpStorm Settings

Image

  1. Create index.js file and open it up in editor
let helloWorld = "Hello World 123";

In the bottom bar you'll briefly see "Starting ESLint Service", after that the Project Status will be Unhealthy

Image

Image

If you try to restart it, this error comes up:

Waiting for containers to become ready: [web db] 
Failed waiting for web/db containers to become ready: web container failed: log=, err=health check timed out after 2m0s: labels map[com.ddev.site-name:eslint-demo com.docker.compose.service:web] timed out without becoming healthy, status=, detail= ddev-eslint-demo-web-run-376219351db8:starting
Troubleshoot this with these commands:
[
        ddev logs -s web
        docker logs ddev-eslint-demo-web-run-376219351db8
        docker inspect --format "{{ json .State.Health }}" ddev-eslint-demo-web-run-376219351db8 | docker run -i --rm ddev/ddev-utilities jq -r
]  
/eslint-demo % 
/eslint-demo % docker logs ddev-eslint-demo-web-run-376219351db8

Error response from daemon: No such container: ddev-eslint-demo-web-run-376219351db8
/eslint-demo % docker inspect --format "{{ json .State.Health }}" ddev-eslint-demo-web-run-376219351db8 | docker run -i --rm ddev/ddev-utilities jq -r

Error: No such object: ddev-eslint-demo-web-run-376219351db8

Additional context

  • Orbstack Version 1.8.2
  • ddev version v1.24.2

Thanks very very much for maintaining the add on!

mandrasch avatar Feb 12 '25 14:02 mandrasch

PS: Example for DDEV user running into this https://youtrack.jetbrains.com/issue/WEB-69291/ESLint-brings-down-all-docker-containers

mandrasch avatar Feb 12 '25 14:02 mandrasch

I guess I've been following the PhpStorm issue on this for some years. Although they've gradually learned how to use php in the container as interpreter, they've never done that with node. It's possible there's some solution here that @AkibaAT might find though?

Thanks for the excellent reproduction discussion.

rfay avatar Feb 12 '25 14:02 rfay

I don't think this has anything to do with this plugin, although I requested this issue. It's a failed/missing implementation by JetBrains. @AkibaAT @nico-loeber let's close it if you don't happen to see a workaround that the plugin could employ, which I doubt.

rfay avatar Feb 12 '25 14:02 rfay

Just a question from the sidelines: Does setting the NodeJS interpreter to use DDEV work in other scenarios / for other use cases? The other comment is about Tailwind breaking the containers. Could we just leave the interpreter setting as it is and provide documentation to people that they need to adjust their local node version to match their desired .ddev/config.yaml nodejs version till Jetbrains fixes this? (Just curious)

mandrasch avatar Feb 12 '25 14:02 mandrasch

IIRC the overall problem is that you can't exec node (using compose) no matter what you do, it always starts a new container.

rfay avatar Feb 12 '25 14:02 rfay

Okay, so that means if the DDEV addon would not change the settings here automatically and would not set the value to DDEV docker compose, it would work just fine for users? (with their local node path set by default)

Image

(This feature of the DDEV add on is called „Auto Configure Node.js Remote Interpreter“)

mandrasch avatar Feb 12 '25 14:02 mandrasch

JFYI: Since other Jetbrains Issues about this were closed, I created a new one WEB-71717

mandrasch avatar Feb 17 '25 18:02 mandrasch

@mandrasch, it's a great report, but I'm afraid it will be ignored because it mentions DDEV so many times.

They will think: no, this is some problem with a third-party plugin, we don't care about that.

I suggest creating a minimal reproducible example that only includes the docker-compose.yaml file with its dependencies so they can reproduce it. I don't think they will install DDEV to test it.

  1. Grab .ddev/.ddev-docker-compose-full.yaml and .webimageBuild/Dockerfile.
  2. Rename .ddev-docker-compose-full.yaml to docker-compose.yaml, remove db service to make it simpler, remove ddev_default network, remove external volumes, make ddev-global-cache an internal volume.
  3. Place Dockerfile next to docker-compose.yaml, adjust web service context to use this Dockerfile.
  4. Use this docker-compose.yaml inside a newly created folder opened in PhpStorm, run docker compose up -d, configure PHP remote interpreter from this docker-compose.yaml.
  5. Try Automatic ESLint Configuration.
  6. Check docker ps -a for the failed web container.

stasadev avatar Feb 17 '25 23:02 stasadev

Great advice. Unless it's boiled down to something that demonstrates nothing but what PhpStorm does, it is unlikely to success.

I was also confused by why you posted this in the Webstorm issue queue instead of PhpStorm or possibly IntelliJ queue.

rfay avatar Feb 18 '25 02:02 rfay

@stasadev thanks very much for the detailed guide, don't know if I can get this done with my rudimentary docker skills. Might be better that someone else reports this in a solid way if my current ticket fails.

I was also confused by why you posted this in the Webstorm issue queue instead of PhpStorm or possibly IntelliJ queue.

yeah, my mistake - I was in the wrong context while reading the other issues. (Btw: I just checked, the ddev plugin can also be installed in WebStorm which is free for non-commercial use. But there is an error on startup - stacktrace).

mandrasch avatar Feb 18 '25 07:02 mandrasch

Automatic ESLint Configuration works for me when I choose Docker Remote Interpreter for Node.js instead of Docker Compose:

Image


In this case, it creates a new Docker container that doesn't have this label com.ddev.site-name:eslint-demo:

Failed waiting for web/db containers to become ready: web container failed: log=, err=health check timed out after 2m0s: labels map[com.ddev.site-name:eslint-demo com.docker.compose.service:web] timed out without becoming healthy, status=, detail= ddev-eslint-demo-web-run-376219351db8:starting


When we use Docker Compose interpreter, this label com.ddev.site-name:<project-name> is added automatically by docker-compose config from the YAML file, and DDEV uses this label to see the health of the containers.

I'm not sure if changing this behavior in DDEV can help in any way, but using the remote Docker interpreter seems to work (at least on Linux).

@mandrasch, could you check if this works for you?

P.S. In the end, I don't think there is a JetBrains related problem (if they removed all the labels from their <our-web-container>-run container, it would work...)

stasadev avatar Feb 18 '25 13:02 stasadev

@stasadev Quick test for setting it to Docker (instead of Docker Compose), I can confirm that the project does not become unhealthy 👍

For testing: Beware that sometimes a restart of the service is needed for robust testing after settings were changed

Image


Optional info, think you're aware of it: I see that an additional container is mounted (which is also correctly destroyed when PhpStorm is closed):

Image

If I also install prettier via ddev npm install --save-dev --save-exact prettier and enable Prettier support in PhpStorm settings with Automatic Configuration and Run on Save as well, there is another container mounted by PhpStorm

Image

I can see in the logs that one container is used for ESlint, the other for prettier. Everything works as expected so far 🥳


mandrasch avatar Feb 18 '25 13:02 mandrasch

The problem with it starting an additional container at all is that it will ignore any DDEV configuration, so for example, the node version may not be right if it's not default.

rfay avatar Feb 18 '25 15:02 rfay

Oh, right, it's using the default nodejs_version (because we install Node.js in start.sh).

So if you need the same version, docker pull node:X, where X is the nodejs_version used by DDEV, and select that node image in PhpStorm Node.js interpreter.

stasadev avatar Feb 18 '25 15:02 stasadev

Well, there are lots of other things that could be different :) It's certainly possible to use other ways to run node.

rfay avatar Feb 18 '25 15:02 rfay

JFYI: I posted about the workaround to use the official nodejs Docker image in order to help users researching this via Google https://dev.to/mandrasch/ddev-and-phpstorms-nodejs-remote-interpreter-for-eslint-prettier-tailwind-etc-1lm6

mandrasch avatar Feb 27 '25 13:02 mandrasch

Jetbrains responded, connecting to existing docker containers is a new feature request (https://youtrack.jetbrains.com/issue/WEB-71717/Setting-Node.js-Remote-Interpreter-to-Docker-Compose-path-crashes-DDEV-projects-Eslint-etc.#focus=Comments-27-11632086.0-0)

From my point of view, removing the automatic setting for the Node Interpreter from the DDEV integration plugin would be the easiest solution for now to avoid crashes.

(The default setting is local node, ESLint works fine with that. If users want to use the same NodeJS Version as in the DDEV project, they can change their local node or use a remote Docker image, see my comment above)

mandrasch avatar Feb 28 '25 13:02 mandrasch

From my point of view, removing the automatic setting for the Node Interpreter from the DDEV integration plugin would be the easiest solution for now to avoid crashes.

I didn't know it was set by this plugin, if so, then I agree, it should be disabled (at least for now).

stasadev avatar Feb 28 '25 13:02 stasadev

@mandrasch,

I have a fix for this in:

  • https://github.com/ddev/ddev/pull/7148

Please test and leave a review.

stasadev avatar Apr 02 '25 12:04 stasadev

FIxed in DDEV v1.24.5.

stasadev avatar May 15 '25 10:05 stasadev