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

chromedriver breaks with pnpm's side-effects cache #372

Open
zkochan opened this issue Jul 16, 2022 · 4 comments
Open

chromedriver breaks with pnpm's side-effects cache #372

zkochan opened this issue Jul 16, 2022 · 4 comments

Comments

@zkochan
Copy link

zkochan commented Jul 16, 2022

You can't do anything about it right now but I wanted to ask your opinion how to best fix this on pnpm's side.

The issue is that pnpm uses side-effects cache for caching builds. So pnpm will not run the post install script of a package twice on the same machine. This works great with most of the packages and makes installations faster. However, chromedriver's scripts have different side-effects when some environment variables are set. Hence, when pnpm caches the result of one build, it might be invalid to reuse the same cache for a different build of chromedriver as it might be executed with other environment variables. Here is an issue that was created as a result of this.

There are a couple of ways we may fix this. I have described the possible solutions in this issue. I think the best solution would be to support some package.json field that would tell pnpm which environment variables may influence the build of chromedriver. In that case, pnpm would be able to save the build cache under a key that would include the values of those env variables. Would you agree to ship such additional metadata for better DX of pnpm users?

@giggio
Copy link
Owner

giggio commented Jul 19, 2022

I don't think adding a field to package.json to fix this specific issue would be a problem. How would that look like?

@zkochan
Copy link
Author

zkochan commented Jul 20, 2022

It would probably be a list of environment variable names that pnpm should include in the cache key. For instance:

{
  "name": "chromedriver",
  "version": "103.0.0",
   "pnpm": {
     "includedEnvVarsInSideEffectsKey": ["detect_chromedriver_version"]
   }
}

So pnpm would use a different cache for chromedriver, when detect_chromedriver_version is set to true.

@giggio
Copy link
Owner

giggio commented Jul 20, 2022

That's fine, please send a PR and I'll merge it.
Also, please send the link to pnpm's docs, so I can get more information about how pnpm will use this, and can better review the PR.

@zkochan
Copy link
Author

zkochan commented Jul 20, 2022

ok, once we decide on a solution and it gets implemented, I'll send a PR. Thanks.

@stale stale bot added the wontfix label Jul 30, 2022
@stale stale bot closed this as completed Aug 12, 2022
@giggio giggio reopened this Aug 12, 2022
Repository owner deleted a comment from stale bot Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants