reactioncommerce / generator-reaction

Project generator for Reaction NodeJS projects. Built with Yeoman.
Other
5 stars 3 forks source link

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

Closed machikoyasuda closed 3 years ago

machikoyasuda commented 6 years ago

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:

aldeed commented 6 years ago

@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 commented 6 years ago

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:

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 commented 6 years ago

@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 commented 6 years ago

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.