Add link checker to Husky pre-commit hook (#840) #854
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's Guide by SourceryThis pull request introduces a link checker using Sequence diagram for the link checking process during commitsequenceDiagram
actor Developer
participant Git
participant Husky
participant LinkChecker
participant HTML
Developer->>Git: git commit
Git->>Husky: trigger pre-commit hook
Husky->>LinkChecker: check staged HTML files
LinkChecker->>HTML: scan for links
HTML-->>LinkChecker: return links
LinkChecker->>LinkChecker: validate links
alt broken links found
LinkChecker-->>Husky: return error (exit 1)
Husky-->>Git: abort commit
Git-->>Developer: show error message
else all links valid
LinkChecker-->>Husky: return success (exit 0)
Husky-->>Git: proceed with commit
Git-->>Developer: commit successful
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
bot
added
the
_community
✅ Deploy Preview for neurobagel-annotator ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
rmanaem
requested changes
Feb 3, 2025
There was a problem hiding this comment.
Thanks for taking this issue on @Ravish990 and congratz on your first contribution to Neurobagel! 🥯
We noticed that some of the broken links are actually in the .vue file so I updated the issue to include checking those as well, please take a look.
Aside from that I left a few review comments please have a look and respond.
Comment on lines
-56
to
-64
|
|
||
| // Disallow line breaks before the closing bracket on a multiline opening tag, i.e. - | ||
| // Yes: <tagname | ||
| // attribute1="" | ||
| // attribute2=""> | ||
| // No : <tagname | ||
| // attribute1="" | ||
| // attribute2="" | ||
| // > |
There was a problem hiding this comment.
Was removal of comments done automatically?
Comment on lines
+4
to
+12
| echo "Running pre-commit checks..." | ||
|
|
||
| # Run the link checker | ||
| echo "Checking links in HTML files..." | ||
| node check-links.js | ||
| LINK_CHECK_EXIT_CODE=$? | ||
|
|
||
| # Run lint-staged | ||
| echo "Running lint-staged..." |
There was a problem hiding this comment.
I don't think this is the right place for this code, we usually run the check/linting commands using lint-staged to apply only to staged files and then use husky to run lint-staged pre-commit.
Comment on lines
+14
to
+31
| LINT_STAGED_EXIT_CODE=$? | ||
|
|
||
| # Check the exit codes of both processes | ||
| if [ $LINK_CHECK_EXIT_CODE -ne 0 ]; then | ||
| echo "Link check failed. Please fix the broken links before committing." | ||
| fi | ||
|
|
||
| if [ $LINT_STAGED_EXIT_CODE -ne 0 ]; then | ||
| echo "Lint-staged failed. Please fix the linting issues before committing." | ||
| fi | ||
|
|
||
| # Abort commit if any checks fail | ||
| if [ $LINK_CHECK_EXIT_CODE -ne 0 ] || [ $LINT_STAGED_EXIT_CODE -ne 0 ]; then | ||
| echo "Pre-commit checks failed. Aborting commit." | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "All checks passed! Proceeding with commit." |
There was a problem hiding this comment.
I don't think this is needed since lint-staged as part of its flow displays the checks that failed/passed.
Comment on lines
+1
to
+47
| const { check } = require('linkinator'); | ||
| const glob = require('glob'); | ||
|
|
||
| async function run(files) { | ||
| // If no staged files are passed, fall back to all HTML files | ||
| if (!files || files.length === 0) { | ||
| console.log('No HTML files staged. Checking all HTML files in the repository...'); | ||
| files = glob.sync('**/*.html', { ignore: 'node_modules/**' }); | ||
|
|
||
| if (files.length === 0) { | ||
| console.log('No HTML files found. Skipping link check.'); | ||
| process.exit(0); | ||
| } | ||
| } | ||
|
|
||
| let allLinksValid = true; | ||
|
|
||
| for (const file of files) { | ||
| console.log(`Checking links in ${file}...`); | ||
| const results = await check({ | ||
| path: file, | ||
| recurse: true, | ||
| concurrency: 5 | ||
| }); | ||
|
|
||
| for (const link of results.links) { | ||
| if (link.state === 'BROKEN') { | ||
| console.error(`❌ Broken link found in ${file}: ${link.url}`); | ||
| allLinksValid = false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (allLinksValid) { | ||
| console.log('✅ All links are valid!'); | ||
| process.exit(0); | ||
| } else { | ||
| console.error('❌ Broken links detected. '); | ||
| process.exit(1); | ||
| } | ||
| } | ||
|
|
||
| const stagedFiles = process.argv.slice(2); | ||
| run(stagedFiles).catch((err) => { | ||
| console.error('Unexpected error:', err); | ||
| process.exit(1); | ||
| }); |
There was a problem hiding this comment.
Thanks for implementing this script to check for broken links but I believe the functionality implemented here is already supported natively by linkinator.
| @@ -38,7 +42,6 @@ | |||
| "eslint-config-airbnb": "^19.0.4", | |||
| "eslint-plugin-cypress": "^3.6.0", | |||
| "eslint-plugin-vue": "^9.32.0", | |||
| "husky": "^9.1.7", | |||
There was a problem hiding this comment.
Why revert husky back to v7?
|
Hey @Ravish990, I'm going to close this PR for now due to inactivity. If you're still interested in working on this, feel free to reopen it or ping us on the related issue, and we'd be happy to discuss your implementation further. Thanks for your contribution! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Broken links were not flagged during commits, leading to outdated or invalid links being introduced into the codebase.
Solution
Integrated linkinator: Checks for broken links in HTML files.
Added check-links.js: Recursively checks all HTML files and reports broken links.
Updated package.json:
Added a check:links script for running the link checker manually.
Configured lint-staged to check .html files during pre-commit.
Configured Husky pre-commit hooks: Blocks commits with broken links.
Using Husky 7 version.
Fixed linting issues in check-links.js.
Added Node.jsenvironment support in .eslintrc.js.
Key Changes
check-links.js: Script to validate HTML links using linkinator.
*package.json: *
Added check:links.
Updated various packages.
Used Husky 7 version.
Added glob and linkinator.
Testing
Tested the link checker locally: Verified with HTML files containing broken and valid links.
Verified pre-commit hook functionality: Ensured broken links block commits while valid links allow commits to proceed.
Summary by Sourcery
Added a link check to the pre-commit hooks, blocking commits with broken links.****