Skip to content

Conversation

@pklingem
Copy link
Member

@pklingem pklingem commented Oct 1, 2018

  • bumps yarn to 1.10.1
  • exposes NODE_MODULES_DIR env var, set to /node_modules which can be used to yarn install --modules-folder=$NODE_MODULES_DIR (yes, I refuse to call it a folder).

yarn 1.10.0 includes a bug fix for a bug preventing using a custom directory for node_modules. This is a feature we've wanted to use because it will allow us to avoid tricky volume issues with the

volumes:
   - '/service/node_modules'

hack, originally described here. If we put node_modules outside of the /service path we can avoid the issues.

To test:

  1. pull this branch
  2. make build_10-alpine
  3. create the following Dockerfile in a clean directory, where the FROM line uses the sha from the Successfully built 83ef3548012b output of the previous command:
FROM 83ef3548012b
USER $SERVICE_USER

COPY --chown=service:service package.json yarn.lock $SERVICE_ROOT/
RUN yarn install --pure-lockfile --modules-folder=$NODE_MODULES_DIR && yarn cache clean

COPY --chown=service:service . $SERVICE_ROOT/
RUN rm -f $SERVICE_ROOT/.npmrc
  1. npm init using all defaults
  2. yarn add leftpad
  3. docker build .
  4. docker run --rm e2c0384e7eaa ls /node_modules where e2c0384e7eaa is the sha output from the previous command
  5. observe leftpad in the output
  6. repeat steps for 8-alpine
  7. place a yarn squirrel in the comment box of this PR and click the Comment button

@pklingem pklingem changed the title Lock yarn to 1.10.1 in 10-alpine image Lock yarn to 1.10.1 in 8-alpine,10-alpine images Oct 1, 2018
@mgreystone
Copy link
Member

mgreystone commented Oct 2, 2018

This is becoming a rabbit hole. I've confirm that this does indeed install dependences to /node_modules. However, in one of our frontend repos, this breaks jest's watch mode, which was the primary use case. Jest can't seem to find the volumed .git directory anymore.

I'm going to try to come up with minimum repro steps to see if its a Jest bug or something with our setup. But it's becoming a time suck, so i don't expect this to move quickly.

@mgreystone
Copy link
Member

...And as @pklingem pointed out, i was using an alpine image which has no git. 🤦‍♂️

Works as expected with the stretch 8 image. Haven't seen any of the missing modules i was seeing before.

@tecnobrat
Copy link
Contributor

@mgreystone sounds like we should install git into those base images? Its needed to install any git-based deps right?

@mgreystone
Copy link
Member

mgreystone commented Oct 2, 2018

@tecnobrat Nah, twas my misunderstanding. Any image that runs jest or anything else that depends on git can either use stretch or bring git into its own image. No reason to dirty the base image.

@tecnobrat
Copy link
Contributor

tecnobrat commented Oct 2, 2018

We'd prefer single base images, stretch was being phased out in exchange for alpine (but alpine has its own issues)

So we're re-evaluating things, but at the end of the day we wanna pick one base image type and stick to it.

@pklingem
Copy link
Member Author

pklingem commented Oct 2, 2018

I'm going to close this PR for now, we can reopen if it's needed. @mgreystone and I were attempting to move the node_modules folder in another repo and ran into issues. Since we won't be using it imminently we can wait for upstream to bump yarn.

@pklingem pklingem closed this Oct 2, 2018
@mloberg mloberg deleted the yarn-1.10.1 branch August 4, 2021 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants