Update dependency marked to v17 - autoclosed#155
Conversation
Pull Request Review: Update dependency marked to v17SummaryThis PR updates the marked markdown parser from v12.0.0 to v17.0.0, a major version jump spanning 5 major versions. Critical Breaking Changes Analysisv16.0.0 Breaking Changes:
v15.0.0 Breaking Changes:
v14.0.0 & v13.0.0 Breaking Changes:
Code Impact AssessmentCurrent Usage (calendar.astro:7-15, 99): Analysis:
CRITICAL ISSUE: Node.js Version MismatchThe CI/CD workflow uses Node.js 18, but marked v16+ requires Node.js 20+. Required Action: Update .github/workflows/azure-static-web-apps-ashy-field-00e5f470f.yml line 29 to use Node.js 20 or higher. Potential Issues
Testing RecommendationsManual testing is critical since there are no automated tests:
Security Considerations
Recommendations Before Merging
SummaryMerge Decision: This is a legitimate dependency update and upgrading to v17 is recommended for security and bug fixes. However, the Node.js version incompatibility must be resolved first. Critical Action: Update GitHub Actions workflow to use Node.js 20 before merging. Once Node.js is updated and testing is complete, this PR can be safely merged. Review generated by Claude Code following repository CLAUDE.md guidelines |
Pull Request Review: Update dependency marked to v17OverviewThis PR updates the Critical Concerns1. Breaking Changes - High PriorityThe upgrade jumps through multiple breaking changes:
Impact on this codebase:
2. Node.js Version Requirementv16 requires Node.js 20+. This should be verified: # Verify Node version in:
# - .nvmrc or .node-version file
# - package.json engines field
# - GitHub Actions workflows
# - Any Docker/deployment configurations3. Testing Gap - Critical
Code Quality AssessmentCurrent marked usage (calendar.astro:7-15): import { marked } from 'marked';
marked.setOptions({
breaks: true, // Convert \n to <br>
gfm: true // Enable GitHub Flavored Markdown
});Good practices:
Concerns:
Security ReviewPositive:
Concerns:
Performance Considerations
RecommendationsBefore Merging:
Test Commands:# Install dependencies
npm install
# Validate data files
npm run validate
# Test build
npm run build
# Preview locally
npm run preview
# Then manually test /calendar pageSuggested Test Cases:
Missing Elements
VerdictWhile the code changes are minimal (just version bumps), the 5 major version jump with significant breaking changes around HTML escaping and renderer behavior requires thorough testing, especially given:
Suggested approach:
References: |
Pull Request Review: Update dependency marked to v17Thank you for this dependency update! I've reviewed the changes and have the following feedback: 🔴 Critical Issues1. Node.js Version Incompatibility The PR upgrades
According to the v16.0.0 release notes:
Action Required: Update all GitHub Actions workflows to use Node.js 20 before merging this PR.
|
Pull Request Review: Update dependency marked to v17OverviewThis PR updates the Critical Breaking Changes & Compatibility Issues1. Node.js Version Requirement
|
Code Review: Update marked dependency to v17This PR updates marked from v12 to v17 (5 major versions). I found critical issues that must be addressed. Critical Issues (BLOCKER)1. Node.js Version Incompatibility
2. Breaking API Changes
3. ESM-only (v16)
Security ConcernsHTML Escaping Changed (v15)
Test Coverage IssuesNo tests found for markdown functionality
Required Actions
VerdictDO NOT MERGE until Node.js version updated to 20+ After Node.js update, this is RECOMMENDED for bug fixes and security, but needs thorough testing. |
Pull Request Review: Update marked to v17SummaryThis PR updates the Critical Issues🚨 Breaking Change: Node.js Version Requirement
# .github/workflows/azure-static-web-apps-ashy-field-00e5f470f.yml
- name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: '20' # Update from 18 to 20
|
Pull Request Review: Update dependency marked to v17SummaryThis PR updates the Code Quality and Best Practices ✅Positive:
Concerns: Potential Bugs or Issues
|
Pull Request Review: Update dependency marked to v17Thank you for this dependency update! I've reviewed the changes and have the following feedback: OverviewThis PR updates the Breaking Changes AnalysisBased on the marked release notes, several critical breaking changes affect this project: 1. Node.js Version Requirement (v16.0.0)
- name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: '20' # Currently set to '18'2. CommonJS Build Removed (v16.0.0)
3. API Changes (v13.0.0 - Renderer Changes)
4. HTML Escaping Changes (v15.0.0)
Code Quality & CompatibilityCurrent Usage in marked.setOptions({
breaks: true,
gfm: true
});
const html = marked.parse(processedDescription);✅ Good news: The API methods used ( Potential Issues & Risks🔴 Critical: Node.js Version Mismatch
🟡 Medium: Large Version Jump
🟡 Medium: No Test Coverage
Security Considerations✅ Positive: The v15.0.0 change to escape HTML in renderers improves XSS protection for user-generated content from RSS feeds. ✅ No Known Vulnerabilities: No security advisories mentioned in the release notes for the versions being skipped. Performance ConsiderationsTesting RecommendationsBefore merging, please test the following:
RecommendationsBefore Merging:
Future Improvements:
ConclusionOverall Assessment: This update includes important breaking changes that require attention before merging. Merge Readiness: Recommended Action:
The upgrade itself is beneficial for security and future compatibility, but needs the prerequisite Node.js version bump to work properly. |
60aa5de to
de99dc2
Compare
Pull Request Review: Update dependency marked to v17SummaryThis PR updates the marked dependency from v12 to v17, a major upgrade spanning 5 major versions with breaking changes. Critical Issues - BLOCKERSNode.js Version Compatibility (BLOCKER)
Module System Changes
Major API ChangesRenderer API (v13): Renderers now get token objects instead of parameters. Current usage only uses default renderer so should not break. HTML Escaping (v15): HTML now escaped in renderers vs tokenizers. Could affect event descriptions with HTML entities. Security ConsiderationsPositive: Many bug fixes for markdown parsing edge cases including backticks in links, emstrong tokens, HTML comments, and strikethrough rendering. Improves security. Testing Required
Missing Test CoverageNo automated tests for markdown parsing. Recommend adding unit tests for formatDescription function. RecommendationsBEFORE MERGING:
AFTER MERGING:
VerdictStatus: BLOCK - DO NOT MERGE YET Infrastructure updates required first. Once Node 20+ is deployed, this upgrade is advisable for bug fixes and security. Priority: Medium-High |
Pull Request Review: Update dependency marked to v17SummaryThis PR updates the marked package from v12.0.0 to v17.0.0, a major version upgrade spanning 5 major versions. Critical Concerns1. Node.js Version Compatibility - BLOCKING ISSUEProblem: The current CI/CD workflows use Node.js 18, but marked v16+ requires Node.js 20+. Evidence:
Required Action: Update all workflow files to use Node.js 20 Files to update:
Breaking Changes Analysis1. ESM Module Changes (v16.0.0)
2. API Changes (v13.0.0 - v17.0.0)The codebase uses marked.setOptions and marked.parse which remain compatible.
3. Renderer Changes (v13.0.0)
Code Quality AssessmentCurrent usage in src/pages/calendar.astro is robust with:
Testing ConsiderationsNo test suite detected. Recommended manual testing:
Performance ConsiderationsPositive impacts expected:
Security ReviewNo security concerns identified. Package is from official markedjs/marked repository. RecommendationDO NOT MERGE YET Prerequisites before merging:
Once prerequisites are met: This is a safe upgrade with significant bug fixes. Overall assessment: Beneficial upgrade, but Node.js 20 requirement must be addressed first. |
Pull Request Review: Update dependency marked to v17SummaryThis PR upgrades the Critical Issues🔴 BREAKING CHANGE: Node.js Version Requirement
🔴 BREAKING CHANGE: Module Format
|
Pull Request Review: Update dependency marked to v17SummaryThis PR updates the Code Quality & Best Practices ✅Positive aspects:
Breaking Changes Analysis
|
Pull Request Review: Update dependency marked to v17SummaryThis PR upgrades the 🚨 Critical Issues1. Node.js Version Incompatibility - BLOCKINGSeverity: Critical Issue:
Evidence from Release Notes:
Action Required: - name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: '20' # Changed from '18'This must be updated in:
2. Breaking Changes in API UsageSeverity: Medium Current Usage in Codebase: Breaking Changes to Review: v13.0.0 - Renderer API Changes:
v14.0.0 - Old Renderer Removed:
v15.0.0 - HTML Escaping Changes:
v16.0.0 - Module Format Changes:
Current Code Impact:
|
Pull Request Review: Update marked from v12 to v17OverviewThis PR updates the 🔴 Critical Issues1. Breaking Changes Not AddressedThe upgrade from v12 to v17 includes several breaking changes that need verification: v16.0.0 Breaking Changes:
v15.0.0 Breaking Changes:
v14.0.0 Breaking Changes:
v13.0.0 Breaking Changes:
Impact on marked.setOptions({
breaks: true,
gfm: true
});
const html = marked.parse(processedDescription);✅ Good news: The basic
|
| Category | Risk Level | Rationale |
|---|---|---|
| Breaking Changes | 🟡 Medium | Simple API usage, but untested across 5 major versions |
| Runtime Errors | 🟢 Low | Good error handling with fallback |
| Rendering Changes | 🟡 Medium | No visual regression tests |
| Security | 🟢 Low | Improved escaping in v15+ |
| Performance | 🟢 Low | Build-time only |
Overall Risk: 🟡 MEDIUM - Acceptable with proper testing
🎯 Verdict
APPROVE with conditions:
This dependency update is reasonable and includes many bug fixes and improvements. The risk is manageable because:
- Simple API usage (no custom renderers/extensions)
- Good error handling with fallback
- Static site generation (errors caught at build time)
- Only one file affected
However, DO NOT merge until:
- Node.js version compatibility verified
- Manual testing completed on calendar page
- Build succeeds without errors
- Visual inspection shows no rendering regressions
📚 Additional Resources
- Marked v17 Release Notes
- Marked v16 Release Notes
- Marked v13 Migration Guide (major renderer changes)
Generated by Claude Code
Pull Request Review: Update dependency marked to v17SummaryThis PR updates the Critical Issues1. Node.js Version Compatibility (BLOCKING)
2. Breaking API ChangesFrom the release notes, marked v13-v17 introduced several breaking changes: v16.0.0 - Major Build Changes:
v13.0.0 - Renderer API Changes:
Impact on codebase:
Code Quality & Best PracticesPositive Aspects ✅
Concerns
|
Pull Request Review: Update marked dependency to v17OverviewThis PR updates the Code Quality & Best Practices ✅Positive Aspects:
Observations:
Breaking Changes Assessment
|
Pull Request Review: Update marked to v17Thank you for the automated dependency update! I've reviewed this PR and have identified some important considerations. OverviewThis PR updates the Critical Issues1. Node.js Version Incompatibility 🔴Issue: Marked v16.0.0+ requires Node.js 20+, but your CI workflows use Node.js 18. Location:
Impact: This will cause build failures in CI/CD pipelines. From marked v16 release notes:
Recommendation: Update all GitHub Actions workflows to use Node.js 20: - name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version: '20'2. API Breaking Changes
|
Pull Request Review: Update marked to v17SummaryThis PR updates the Code Quality & Best Practices ✅Positive aspects:
Breaking Changes Analysis
|
de99dc2 to
b4663ce
Compare
PR Review: Update dependency marked to v17SummaryThis PR updates Code Quality & Best PracticesCurrent Usage Analysis The codebase uses
This is a straightforward usage pattern that should be compatible with v17. Breaking Changes AssessmentCritical Changes to Evaluate:
Potential IssuesHigh Priority
Medium Priority
RecommendationsBefore Merging:
Test CoverageTesting Recommendations:
Security
Performance
VerdictConditionally Approve - This PR is likely safe to merge given the simple usage pattern, but should be tested locally first. The main risk is the Node.js 20+ requirement. Action Items:
🤖 Generated with Claude Code |
b4663ce to
964a4f7
Compare
964a4f7 to
584cbed
Compare
Pull Request Review: Update dependency marked to v17SummaryThis PR updates the Code Quality & Best Practices✅ Positive:
Breaking Changes AnalysisBased on the release notes, here are the critical breaking changes from v12 to v17: 1. Node.js Version Requirement (v16.0.0)
2. CommonJS Removal (v16.0.0)
3. Renderer API Changes (v13.0.0)
4. List Token Structure Changes (v17.0.0)
5. HTML Escaping Changes (v15.0.0)
Current Usage AnalysisThe Current implementation: // Lines 12-15: Configuration // Line 99: Usage Analysis:
Potential Issues1. Type SafetyLocation: ```typescript ✅ Good: Code already handles non-string return values 2. HTML Rendering ChangesWith the HTML escaping changes in v15, event descriptions containing HTML may render differently:
3. List RenderingChanges to list token structure in v17 may affect:
Security Considerations✅ Positive Security Changes:
Performance Considerations📊 Performance Impact: Likely minimal
Test Coverage❌ Critical Gap: No automated tests
RecommendationsBefore Merging:
After Merging:
Future Improvements:
Approval Recommendation
The current usage is simple enough that breaking changes should have minimal impact, but the lack of automated tests means manual verification is essential. References
|
584cbed to
7c799af
Compare
Pull Request Review: Update dependency marked to v17OverviewThis PR updates the Code Quality & Best Practices ✅Positive aspects:
Breaking Changes Analysis
|
7c799af to
40a29d2
Compare
Pull Request Review: Update dependency marked to v17OverviewThis PR updates the Code Quality & Best Practices ✅Positive aspects:
Critical Issues 🚨1. Node.js Version Incompatibility
|
40a29d2 to
970825f
Compare
970825f to
7250375
Compare
7250375 to
744272d
Compare
e617367 to
c02cfba
Compare
a3738fe to
abc51aa
Compare
abc51aa to
edefae4
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://ashy-field-00e5f470f-155.eastus2.6.azurestaticapps.net |
edefae4 to
51d5a5b
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://ashy-field-00e5f470f-155.eastus2.6.azurestaticapps.net |
51d5a5b to
c1523b3
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://ashy-field-00e5f470f-155.eastus2.6.azurestaticapps.net |
c1523b3 to
23a1875
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://ashy-field-00e5f470f-155.eastus2.6.azurestaticapps.net |
23a1875 to
8107dc1
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://ashy-field-00e5f470f-155.eastus2.6.azurestaticapps.net |
This PR contains the following updates:
^12.0.0→^17.0.0Release Notes
markedjs/marked (marked)
v17.0.6Compare Source
Bug Fixes
v17.0.5Compare Source
Bug Fixes
v17.0.4Compare Source
Bug Fixes
v17.0.3Compare Source
Bug Fixes
v17.0.2Compare Source
v17.0.1Compare Source
Bug Fixes
v17.0.0Compare Source
Bug Fixes
BREAKING CHANGES
v16.4.2Compare Source
v16.4.1Compare Source
Bug Fixes
v16.4.0Compare Source
Bug Fixes
wwwcase-sensitive when detecting link starts (#3770) (626d961)Features
v16.3.0Compare Source
Features
v16.2.1Compare Source
Bug Fixes
v16.2.0Compare Source
Features
v16.1.2Compare Source
Bug Fixes
v16.1.1Compare Source
Bug Fixes
v16.1.0Compare Source
Features
v16.0.0Compare Source
Bug Fixes
BREAKING CHANGES
minify ./lib/marked.esm.js and ./lib/marked.umd.js
remove ./marked.min.js use ./lib/marked.umd.js instead
remove ./lib/marked.cjs
update minimum supported node version to 20 to support
require('marked.esm.js'). see https://nodejs.org/docs/latest-v20.x/api/modules.html#loading-ecmascript-modules-using-requireThis seems to break Jest since it doesn't work with esm by default.
You need to transform marked with:
see Jest docs for more info
v15.0.12Compare Source
Bug Fixes
v15.0.11Compare Source
Bug Fixes
v15.0.10Compare Source
Bug Fixes
v15.0.9Compare Source
Bug Fixes
v15.0.8Compare Source
Bug Fixes
v15.0.7Compare Source
Bug Fixes
v15.0.6Compare Source
Bug Fixes
v15.0.5Compare Source
Bug Fixes
v15.0.4Compare Source
Bug Fixes
v15.0.3Compare Source
Bug Fixes
v15.0.2Compare Source
Bug Fixes
v15.0.1Compare Source
Bug Fixes
v15.0.0Compare Source
Bug Fixes
BREAKING CHANGES
v14.1.4Compare Source
Bug Fixes
v14.1.3Compare Source
Bug Fixes
v14.1.2Compare Source
Bug Fixes
v14.1.1Compare Source
Bug Fixes
v14.1.0Compare Source
Bug Fixes
Features
v14.0.0Compare Source
Bug Fixes
BREAKING CHANGES
async: falseis set when an extension setsasync: truev13.0.3Compare Source
Bug Fixes
v13.0.2Compare Source
Bug Fixes
v13.0.1Compare Source
Bug Fixes
v13.0.0Compare Source
Bug Fixes
Features
BREAKING CHANGES
Most extensions that update marked renderers should still work with this version but will break in a future major version.
Extensions that change marked renderers will need to be updated and use new option
useNewRendererand accept a token object instead of multiple parameters. See updated Renderer docsConfiguration
📅 Schedule: Branch creation - "every weekend" in timezone America/New_York, Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about this update again.
This PR was generated by Mend Renovate. View the repository job log.