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

Inaccurate path to @fastify/pre-commit/hook #32

Open
2 tasks done
qu0cster opened this issue Jul 31, 2021 · 5 comments
Open
2 tasks done

Inaccurate path to @fastify/pre-commit/hook #32

qu0cster opened this issue Jul 31, 2021 · 5 comments

Comments

@qu0cster
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure it has not already been reported

Fastify version

0.0.0

Plugin version

2.0.2

Node.js version

10.19.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

ubuntu 20.04

Description

This fixes the installation to correctly find the .git folder to install...when running "npm install" in a subfolder:
#17

.git/
web/
web/node_modules
web/package.json
...

but if you look at the pre-commit it executing:
./node_modules/@fastify/pre-commit/hook

that should be
./web/node_modules/@fastify/pre-commit/hook

Steps to Reproduce

  1. create a folder "web" same level as .git
  2. cd web
  3. npm install --save-dev @fastify/pre-commit
  4. test a commit - errors
  5. cat .git/hooks/pre-commit
  6. see './node_modules/@fastify/pre-commit/hook' instead of './web/node_modules/@fastify/pre-commit/hook"

Expected Behavior

expect to see './web/node_modules/@fastify/pre-commit/hook' instead of './node_modules/@fastify/pre-commit/hook"

@jsumners
Copy link
Member

We'd welcome a PR. But I doubt it will be easy. The module is written for plain projects, not monorepos.

@qu0cster
Copy link
Author

qu0cster commented Aug 1, 2021

@jsumners should just be one line change in install.js line 99

// locate the hook relative from the root of project - support subfolders
let hookRelativeUnixPath = hook.replace(path.resolve(git, '..'), '.')

@mcollina
Copy link
Member

mcollina commented Aug 1, 2021

@jsumners should just be one line change in install.js line 99


// locate the hook relative from the root of project - support subfolders

let hookRelativeUnixPath = hook.replace(path.resolve(git, '..'), '.')

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@qu0cster
Copy link
Author

qu0cster commented Aug 3, 2021

Sure I will do it when I have time;

@Cyclodex
Copy link

I can just confirm that above change of line 99 helps to fix the path for the hook file execution, but the pre-commit will then fail with :
Error: Cannot find module '@fastify/pre-commit'

I had a further idea, to first go into the subfolder before executing the hook:

#!/usr/bin/env bash
cd ./web
./node_modules/@fastify/pre-commit/hook
...

I get 1 step further but now I have an other error:

pre-commit:
pre-commit: Received an error while parsing or locating the `package.json` file:
pre-commit:
pre-commit:   Cannot find module 'C:\abc\projectX\package.json'

So, we miss the correct place of the package.json now again, this definitely needs more changes to bring it to work with subfolder scenarios...

So on the following lines, you would need to fix the path as well:

try {
    this.json = require(path.join(this.root, 'package.json'))
    this.parse()
  } catch (e) { return this.log(this.format(Hook.log.json, e.message), 0) }

I don't have time to fix this here, but just wanted to share my findings. Cheers!

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

No branches or pull requests

4 participants