Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Linting doesn't work out-of-the-box for new developers #8

Closed
machikoyasuda opened this issue May 16, 2018 · 4 comments
Closed

Linting doesn't work out-of-the-box for new developers #8

machikoyasuda opened this issue May 16, 2018 · 4 comments

Comments

@machikoyasuda
Copy link

When I first ran the generator to create a basic front-end app, I got several errors when I tried to run docker-compose run web yarn run lint.

The package.json file did not have any of the devDependencies I needed to get eslint running. I got errors indicated required modules were missing, like: [Error - 13:54:51] Cannot find module 'babel-eslint' Referenced from: /Users/machikoyasuda/Desktop/reaction-static/package.json

Here is how I got linting working for me locally:

# rename .yarnrc
yarn add @reactioncommerce/eslint-config --dev
npx install-peerdeps --dev @reactioncommerce/eslint-config
# restart VSCode
yarn add babel-eslint --dev
# restart VSCode
# magically started working
# unrename .yarnrc
docker-compose run web yarn run lint
# test runs

In my generated app, I ran all of these things so that the package.json file has all the devDependencies listed and CircleCI was able to build it and run yarn run lint itself: https://github.com/reactioncommerce/reaction-static/pull/16

To improve dev experience, I think we should:

  • Add these docs to the generator README, or,
  • Make sure the generator generated app has these necessary linting tools built in it
@aldeed
Copy link
Contributor

aldeed commented May 16, 2018

@ticean Any thoughts about how we can best have that .yarnrc config apply within containers but not outside? I think this was initially fine because local node_modules was linked into the container, but now that we use a separate volume for node_modules, there are some dev deps that need to be installed outside of docker for IDEs to pick them up (primarily eslint)

@ticean
Copy link
Member

ticean commented May 16, 2018

The .yarnrc config prevents yarn from working outside the Docker container. IMO a good thing.

When one runs npm|npx|yarn install from the development host machine then the node_modules dependency directory is created on the host. Then it is always mounted into the Docker container as long as it exists. There's a few problems with that:

  • Runtime compatibility issues (C-bindings built on Mac that fail to run in Linux)
  • "Works on my machine" because manually installed development dependencies out of sync with what's actually in the committed code. Container doesn't match image.

Both tricky problems to debug. I think it's best to prevent it completely and embrace the remote Docker environment.


It seems the hurdle is that @machikoyasuda wants her IDE to use the project's runtime for linting, but that runtime is remote. Here are some options:

a. Global Linting Dependencies

Don't depend on the project runtime. Install the dependencies in the global development environment.

b. Use the Containerized Environment

a1) Modify the command script that the IDE uses to run the lint in a Dockerized command. That should be fairly easy to do.

a2) Or, treat this similarly to remote debugging. Is there a plugin or technique that will attach to the remote environment? That would probably require some assumption on IDE choice as it may introduce project files to support it.

c. Load Dependencies from Outside the Project Directory

Alternatively, the same trick that we use inside the container could work in the development environment.

Node searches up the directory tree to find a node_modules, so you could install dependencies in a parent directory on the development machine.

cp package.json ../
cd ..
npm install
# cd back to project and do work...

@aldeed
Copy link
Contributor

aldeed commented May 17, 2018

@ticean Are you sure host node_modules is mounted into the container? Since container node_modules is linked with a volume, it doesn't seem like anything in host node_modules has any effect in the container. e.g. If I npm install something into host node_modules, app running in the container will still complain that it isn't there.

@ticean
Copy link
Member

ticean commented May 18, 2018

Oh, right @aldeed. I forgot about that change with the mounted volume. Thanks. The hard paths in the yarn.rc directives are the only things that prevent it from working on the host. I haven't tested but possible that interpolated vars like $HOME or ~ might work. You could always leave those out completely. I do remember seeing some significant performance boosts with the caching but not sure how that was affected with the changes to the mount.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants