Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Deps] Upgrade ast-types-flow to mitigate Docker user namespacing problems #930

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

rmehner
Copy link
Contributor

@rmehner rmehner commented Apr 18, 2023

Version 0.0.7 had a problem with a very high user id, which lead to problems when used in Docker with user namespacing. For some details check: kyldvs/ast-types-flow#5

…blems

Version 0.0.7 had a problem with a very high user id, which lead to problems when used in Docker with user namespacing.
For some details check: kyldvs/ast-types-flow#5
@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #930 (f0d2ddb) into main (db64898) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #930   +/-   ##
=======================================
  Coverage   99.29%   99.29%           
=======================================
  Files         104      104           
  Lines        1554     1554           
  Branches      522      522           
=======================================
  Hits         1543     1543           
  Misses         11       11           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is a dev dep tho, so i'm not sure why it would be an issue for anyone who's not actively maintaining this package :-)

@ljharb ljharb merged commit f0d2ddb into jsx-eslint:main Apr 18, 2023
@rmehner
Copy link
Contributor Author

rmehner commented Apr 18, 2023

Thanks! This is a dev dep tho, so i'm not sure why it would be an issue for anyone who's not actively maintaining this package :-)

Thanks for the quick merge. Unfortunately in one of my client projects the dev dependencies end up in the container's intermediate layers to allow the build to happen within the Dockerfile, so this fixes it :)

@rmehner rmehner deleted the chores/bump-ast-types-flow-version branch April 18, 2023 16:42
@ljharb
Copy link
Member

ljharb commented Apr 18, 2023

That's surprising since there's no build needed - the build output is part of the npm package already.

@rmehner
Copy link
Contributor Author

rmehner commented Apr 19, 2023

That's surprising since there's no build needed - the build output is part of the npm package already.

Sorry for being unclear. In this case it's another project that depends on eslint-plugin-jsx-a11y which depends on ast-types-flow and this project is trying to install dev dependency within docker, which leads to the aforementioned problem.

@ljharb
Copy link
Member

ljharb commented Apr 19, 2023

I’m still unclear on how that’s possible unless they’re incorrectly running npm install --dev - npm doesn’t install transitive dev deps by default.

@rmehner
Copy link
Contributor Author

rmehner commented Apr 19, 2023

Dockerfile:

FROM node:18.16.0-slim

COPY package*.json ./

RUN npm ci

and the package.json:

{
  "devDependencies": {
    "eslint-plugin-jsx-a11y": "^6.7.1"
  }
}

And since I didn't have access to the Dockerfile for reasons, this has been the best option.

@ljharb
Copy link
Member

ljharb commented Apr 19, 2023

I'd suggest making a new lockfile from scratch; dev deps of your dev deps (or deps) should never be installed.

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.

2 participants