fix: Proper way of finding `npm pack` `filename` output. by ovcharenko · Pull Request #3460 · pre-commit/pre-commit · GitHub

That will fix pollution of installation path with TypeScript output:

E           pre_commit.util.CalledProcessError: command: ('/private/var/folders/sd/qk6llt717b99lp1wgtym5qwm0000gn/T/pytest-of-local.user/pytest-16/test_node_typescript_hook_syst0/node_env-system/bin/node', '/opt/homebrew/bin/npm', 'install', '-g', '/private/var/folders/sd/qk6llt717b99lp1wgtym5qwm0000gn/T/pytest-of-local.user/pytest-16/test_node_typescript_hook_syst0/> t@0.0.1 prepare\n> tsc\n\nt-0.0.1.tgz')
E           return code: 254
E           stdout: (none)
E           stderr:
E               npm warn tarball tarball data for file:/private/var/folders/sd/qk6llt717b99lp1wgtym5qwm0000gn/T/pytest-of-local.user/pytest-16/test_node_typescript_hook_syst0/> t@0.0.1 prepare> tsct-0.0.1.tgz (null) seems to be corrupted. Trying again.
E               npm warn tarball tarball data for file:/private/var/folders/sd/qk6llt717b99lp1wgtym5qwm0000gn/T/pytest-of-local.user/pytest-16/test_node_typescript_hook_syst0/> t@0.0.1 prepare> tsct-0.0.1.tgz (null) seems to be corrupted. Trying again.
E               npm error code ENOENT
E               npm error syscall open
E               npm error path /private/var/folders/sd/qk6llt717b99lp1wgtym5qwm0000gn/T/pytest-of-local.user/pytest-16/test_node_typescript_hook_syst0/> t@0.0.1 prepare> tsct-0.0.1.tgz
E               npm error errno -2
E               npm error enoent ENOENT: no such file or directory, open '/private/var/folders/sd/qk6llt717b99lp1wgtym5qwm0000gn/T/pytest-of-local.user/pytest-16/test_node_typescript_hook_syst0/> t@0.0.1 prepare> tsct-0.0.1.tgz'
E               npm error enoent This is related to npm not being able to find a file.
E               npm error enoent
E               npm error A complete log of this run can be found in: /Users/local.user/.npm/_logs/2025-05-09T10_21_58_861Z-debug-0.log

the other branch which tried to solve this is here and my recommendation: #3259 (comment)

the other branch which tried to solve this is here and my recommendation: #3259 (comment)

That doesn't help much IMHO, because the JSON is part of other noisy output. So instead of one-line fix you will have to find JSON object and handle all error parsing.

pretty sure in json mode it does a better job of obscuring the nonsense

pretty sure in json mode it does a better job of obscuring the nonsense

19d0b42 should do this, right?

@asottile any chance to get this merged any soon?

this is going to make the test prohibitively slow -- can you make a faster example?

@asottile I took an example from #3259 instead. Will that work?

new code should not end up in utils -- also this is only used in one place

this seems unrelated to your patch / not necessary

just let it raise no reason to catch and reraise

I would just let these raise

rather than changing this function -- call it and then write a package.json in the specific test below

can you show the output of this failing before your patch?

via #3719

ovcharenko mentioned this pull request May 9, 2025

feat: Add pre-commit hooks. npalm/action-docs#830

Open

ovcharenko changed the title fix: Process only the last line of npm pack output. fix: Proper way of finding npm pack filename output. May 9, 2025

asottile reviewed May 23, 2025

View reviewed changes

asottile mentioned this pull request Sep 6, 2025

Support lifecycle scripts for node #3259

Closed

ovcharenko force-pushed the main branch 4 times, most recently from bbabec2 to 928edc0 Compare September 26, 2025 15:51

fix: Use JSON to parse npm pack output.

24d595f

ovcharenko force-pushed the main branch from 404ec8b to 24d595f Compare September 26, 2025 15:57

asottile reviewed Oct 2, 2025

View reviewed changes

asottile closed this Jul 15, 2026

+              def get_npm_version() -> tuple[int, ...]:
+                  _, out, _ = cmd_output('npm', '--version')
+                  version_match = re.match(r'^(\d+)\.(\d+)\.(\d+)', out)
+                  if version_match is None:
+                      return 0, 0, 0
+                  else:
+                      return tuple(map(int, version_match.groups()))

+                      if not pkg_json:
+                          raise ValueError('JSON array from npm pack is empty.')
+                      if not isinstance(pkg_json, list):
+                          raise ValueError('Expected npm pack output to be a JSON array.')
+                      filename = pkg_json[0].get('filename')
+                      if filename is None:
+                          raise KeyError(
+                              "Key 'filename' not found in the first element "
+                              'of the JSON array.',
+                          )

+                "devDependencies": {
+                  "typescript": "^5.3.0"
+                }

+                      # https://docs.npmjs.com/cli/v11/using-npm/changelog#1100-pre0-2024-11-26
+                      if npm_version >= (11, 0, 0):
+                          args.append('--ignore-scripts')

		except json.JSONDecodeError as e:
		raise ValueError('Failed to parse npm pack output as JSON.') from e

		def _make_hello_world(tmp_path, package_json=None):
		package_json = package_json or '''\

		assert ret == (0, b'Hello World\n')


		def test_node_with_prepare_script(tmp_path):

fix: Proper way of finding `npm pack` `filename` output. by ovcharenko · Pull Request #3460 · pre-commit/pre-commit · GitHub

Uh oh!

Uh oh!

fix: Proper way of finding npm pack filename output. - #3460

fix: Proper way of finding npm pack filename output.#3460ovcharenko wants to merge 1 commit intopre-commit:mainpre-commit/pre-commit:mainfrom ovcharenko:mainovcharenko/pre-commit:mainCopy head branch name to clipboard

Conversation

ovcharenko commented May 9, 2025

Uh oh!

asottile commented May 9, 2025

Uh oh!

ovcharenko commented May 9, 2025

Uh oh!

asottile commented May 9, 2025

Uh oh!

ovcharenko commented May 9, 2025

Uh oh!

ovcharenko commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asottile commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

fix: Proper way of finding npm pack filename output.#3460
ovcharenko wants to merge 1 commit into
pre-commit:mainfrom
ovcharenko:main