Skip to content

Conversation

@stevenjoezhang
Copy link
Member

check list

  • Add test cases for the changes.
  • Passed the CI test.

Description

Issue resolved: #28

See also hexojs/hexo#4548

This pull request introduces significant changes to timezone handling, aiming to address long-standing timezone issues in Hexo. (Of course, after hexo-front-matter is updated, Hexo’s code will also need corresponding adjustments — for example, passing the timezone parameter when calling hexo-front-matter, and prompting users to properly configure the timezone in _config.yml)

The core logic can be found in the comments of lib/timestamp.ts. Specifically:
1. If a timezone is set in the timestamp, use that timezone.
2. Otherwise, use the timezone defined in the Hexo configuration. hexo-front-matter no longer read the timezone from the machine, as this can cause issues in CI environments or when collaborators are in different timezones.
3. If no timezone is provided, treat the timestamp as UTC by default and take no further action.

Note that this behavior differs from the YAML specification — YAML treats unspecified timestamps as UTC, whereas we apply Hexo’s configured timezone.

To support the new functionality, I used the yaml and luxon libraries, which are different from Hexo’s current dependencies (js-yaml and moment-timezone). However, all unit tests have passed, so I believe this should not cause any issues.

Additional information

@stevenjoezhang stevenjoezhang changed the title Yaml Fix timezone issue Apr 20, 2025
@coveralls
Copy link

coveralls commented Apr 20, 2025

Coverage Status

coverage: 98.917% (-1.1%) from 100.0%
when pulling ecccf2e on yaml
into 70426c8 on master.

@stevenjoezhang
Copy link
Member Author

stevenjoezhang commented Apr 21, 2025

To test it with Hexo, checkout this branch: https://github.com/hexojs/hexo/tree/timezone-dev (I'll open a Pull Request once this merged)

@stevenjoezhang stevenjoezhang requested a review from a team April 23, 2025 06:02
@stevenjoezhang stevenjoezhang requested a review from Copilot July 25, 2025 03:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request addresses timezone handling issues in hexo-front-matter by implementing a comprehensive timezone solution. The changes replace the existing date parsing logic with a more robust system that respects explicit timezone information and provides fallback mechanisms.

  • Replaces js-yaml with the yaml library and adds luxon for better timezone handling
  • Implements custom timestamp parsing that respects timezone information from multiple sources
  • Updates test cases to validate timezone behavior across different scenarios

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

File Description
package.json Updates dependencies from js-yaml to yaml and adds luxon for timezone handling
lib/timestamp.ts Implements custom timestamp factory with timezone-aware parsing logic
lib/front_matter.ts Integrates custom timestamp parsing and updates YAML processing
test/index.ts Replaces old date test with comprehensive timezone test cases
Comments suppressed due to low confidence (1)

lib/front_matter.ts:144

  • Add TypeScript type annotations for the parameters. The options parameter should be typed consistently with the StringifyOptions interface or a subset of yaml library options.
    }

Copy link
Member

@yoshinorin yoshinorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think this looks good to merge, but I'd like to hear other maintainers' opinions as well.

lib/timestamp.ts Outdated
@@ -0,0 +1,91 @@
// https://github.com/eemeli/yaml/blob/main/src/schema/yaml-1.1/timestamp.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO:

I think it would be better to use the commit hash from when the code was copied (v2.7.1) instead of pointing to main.

"dependencies": {
"js-yaml": "^4.1.0"
"luxon": "^3.6.1",
"yaml": "2.7.1"
Copy link
Member

@yoshinorin yoshinorin Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question:

Why is only the yaml package pinned to a specific version? I assume it's because you copied the source code from that version—is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since I modified parts of the timestamp parsing code, to avoid compatibility issues, I prefer to use exactly the same version. Later, when we upgrade the yaml dependency, we can rely on unit tests to determine whether it remains compatible.

@stevenjoezhang stevenjoezhang merged commit 1b948db into master Nov 4, 2025
29 of 31 checks passed
@stevenjoezhang stevenjoezhang deleted the yaml branch November 4, 2025 03:48
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

Successfully merging this pull request may close these issues.

Date parse issue

4 participants