chore: 迁移日期时间工具库至luxon#336
Open
YufJi wants to merge 28 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough将项目的日期/时间实现从 moment/moment-timezone 与 date-fns 迁移到 Luxon，新增基于 Luxon 的日期工具模块，重构格式化/解析与初始化流程，并在大量测试中替换全局引用与断言以匹配新实现（无外部交互流程变更）。 Changes
Sequence Diagram(s)（无） Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@packages/basic/src/sdk/modules/utils.ts`:
- Around line 134-137: The startOfWeek function ignores the options.weekStartsOn
argument; update startOfWeek to respect options.weekStartsOn (defaulting to the
same value used by getWeek) by computing the weekday difference and subtracting
that many days from the given date rather than calling Luxon’s startOf('week')
(which always uses Monday). Locate the startOfWeek function and replace the
Luxon startOf call with logic that reads options?.weekStartsOn, computes diff =
(currentDay - weekStartsOn + 7) % 7, subtracts diff days from the input Date
(using Date or DateTime arithmetic), and returns the resulting Date so the
function honors weekStartsOn.
- Around line 126-128: getWeek currently ignores the options parameter; if
options.weekStartsOn is provided we must compute the week number relative to
that start day instead of always using ISO weeks. Change getWeek to: create a
Luxon DateTime from the input, and if options?.weekStartsOn is set compute an
adjustment using DateTime.weekday and the desired week start (treat
weekStartsOn: 0 as Sunday which maps to ISO weekday 7), shift the DateTime
backward by that many days (so that the target week start aligns with ISO
Monday), then return the shifted DateTime.weekNumber; if options is not provided
just return DateTime.fromJSDate(date).weekNumber. Use the function name getWeek,
DateTime.fromJSDate, and DateTime.weekday to locate and implement the fix.
In `@packages/basic/tests/init/utils/date-time/fromString.spec.js`:
- Around line 15-30: The tests in the DateTime parsing suite hardcode a +08:00
offset and will fail in other timezones; update the three tests that call
fromString('...', 'nasl.core.DateTime') to assert the timezone generically
instead of a fixed string: either match the result with a regex for an ISO
datetime with any offset, or compute the expected offset at runtime (e.g.,
derive new Date().getTimezoneOffset() and format the expected suffix) and
compare that, ensuring you refer to the fromString calls in the "should parse
standard datetime string", "should parse ISO datetime string", and "should parse
date-only string to DateTime" tests when making the change.
In `@packages/basic/tests/init/utils/date-time/toString.spec.js`:
- Around line 65-89: The tests hardcode expected DateTime outputs assuming the
runner is in Asia/Shanghai (UTC+8), causing failures in other CI timezones;
update the tests that call toString (tests around toString('nasl.core.DateTime',
...)) to compute expected values dynamically or to assert using an explicit
timezone: either (A) call toString with an explicit timezone argument (e.g.,
'Asia/Shanghai' or 'UTC') and assert the known result, or (B) compute the
expected string at runtime from the input UTC value using
Intl.DateTimeFormat/Date with the same timezone logic used by toString and
compare against that computed value; apply this change to the "default timezone"
and "local datetime string" tests so they no longer rely on the test process TZ.
In `@packages/basic/tests/sdk/utils-luxon.spec.js`:
- Around line 48-59: The tests for sdkUtils.CurrDateTime are too loose (only
checking string), so update them to validate the returned value is a valid ISO
datetime and that the timezone argument actually affects the result: parse the
result with Luxon (DateTime.fromISO(result, { setZone: true })) and assert
.isValid is true for the default case; for the timezone case call
DateTime.now().setZone('Asia/Shanghai') to get the expected offset and assert
DateTime.fromISO(result, { setZone: true }).offset equals that expected offset
(or otherwise compare zone-aware offsets) to ensure the 'Asia/Shanghai' argument
is applied. Ensure both tests reference sdkUtils.CurrDateTime.
- Around line 128-144: The tests for GetSpecificDaysOfWeek use an if
(result.length > 0) guard which masks failures; replace those guards with
explicit assertions that the result is non-empty (e.g.,
expect(result.length).toBeGreaterThan(0)) and then assert the expected
value/format of result[0]; update both tests (the datetime case checking exact
'2024-01-01T00:00:00.000+08:00' and the date case checking
/^\d{4}-\d{2}-\d{2}$/) so they fail when GetSpecificDaysOfWeek returns an empty
array.
🧹 Nitpick comments (10)
packages/basic/src/sdk/Formatters/DateFormatter.ts (1)
4-6: 过时的 TODO 注释应该移除或更新。该注释建议使用 moment 或其他库，但现在已经在使用 Luxon 了。建议更新或移除此注释以避免混淆。
🔧 建议的修改
-/** - * `@TODO`: use moment or some other library - */ +// Date formatting utilities using Luxon for ISO patternspackages/basic/LUXON_MIGRATION.md (1)
94-107: 建议为代码块添加语言标识符。静态分析工具检测到第 94-107 行的状态标记部分缺少语言标识符。虽然这些不是真正的代码块（只是状态文本），但如果保持当前格式，建议添加
text或移除代码块格式。markdown fenced code block best practices for plain textpackages/basic/tests/sdk/utils-date-fns-migration.spec.js (2)
267-274: 存在未使用的变量。
monday和tuesday变量声明后未被使用，应该移除或实际使用它们进行断言。🔧 建议的修改
describe('date-fns 迁移到 Luxon - 辅助函数验证', () => { test('isMonday', () => { - const monday = new Date('2024-01-01'); // Monday - const tuesday = new Date('2024-01-02'); // Note: isMonday 等函数在内部使用，这里通过 GetSpecificDaysOfWeek 间接测试 const result = utils.GetSpecificDaysOfWeek('2024-01-01', '2024-01-07', [1]); // Monday expect(result.length).toBe(1); });
340-346: 性能测试的断言没有实际意义。
expect(true).toBe(true)不验证任何实际行为。如果要保留性能测试，建议添加有意义的断言或移除这个测试用例。🔧 建议的修改
test('should handle multiple DateDiff calculations', () => { + const start = Date.now(); for (let i = 0; i < 100; i++) { utils.DateDiff('2020-01-01', '2024-12-31', 'd'); } - // If this test passes, performance is acceptable - expect(true).toBe(true); + const elapsed = Date.now() - start; + // 100 次计算应在合理时间内完成 + expect(elapsed).toBeLessThan(500); });packages/basic/src/sdk/modules/utils.ts (2)
313-326: 重复的日期解析逻辑可提取为辅助函数
JsonSerialize中存在三处几乎相同的日期解析回退逻辑。建议提取为一个辅助函数以减少代码重复。♻️ 建议的重构
+function parseToDateTime(v: string, zone: string): DateTime { + let dt = DateTime.fromISO(v, { zone }); + if (!dt.isValid && typeof v === 'string') { + dt = DateTime.fromFormat(v, 'yyyy-MM-dd HH:mm:ss', { zone }); + } + if (!dt.isValid && typeof v === 'string') { + dt = DateTime.fromFormat(v, 'yyyy-MM-dd HH:mm:ss.SSS', { zone }); + } + return dt; +} JsonSerialize(v, tz?) { if (isInputValidNaslDateTime(v)) { if (!tz) { - let dt = DateTime.fromISO(v, { zone: 'UTC' }); - if (!dt.isValid && typeof v === 'string') { - dt = DateTime.fromFormat(v, 'yyyy-MM-dd HH:mm:ss', { zone: 'UTC' }); - } - if (!dt.isValid && typeof v === 'string') { - dt = DateTime.fromFormat(v, 'yyyy-MM-dd HH:mm:ss.SSS', { zone: 'UTC' }); - } - const d = dt.isValid - ? dt.toFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZZ") - : DateTime.fromJSDate(safeNewDate(v), { zone: 'UTC' }).toFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); + const dt = parseToDateTime(v, 'UTC'); + const d = dt.isValid + ? dt.toFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZZ") + : DateTime.fromJSDate(safeNewDate(v), { zone: 'UTC' }).toFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); return JSON.stringify(d); } // ... similar refactoring for other branches } }Also applies to: 331-341, 346-356
1575-1593:ConvertTimezone函数现在是空操作函数验证了输入但直接返回原值，未进行任何时区转换。注释说明这是为了兼容老版本升级脚本，但当前实现可能会让调用者困惑。建议添加更明确的文档说明或考虑抛出弃用警告。
ConvertTimezone(dateTime, tz) { if (!dateTime) { this.helpers.throwError(`内置函数ConvertTimezone入参错误：指定日期为空`); } if (!isValid(safeNewDate(dateTime))) { this.helpers.throwError(`内置函数ConvertTimezone入参错误：指定日期不是合法日期类型`); } if (!isValidTimezoneIANAString(tz)) { this.helpers.throwError(`内置函数ConvertTimezone入参错误：传入时区${tz}不是合法时区字符`); } + // 注意：此函数为了向后兼容保留，但不再执行实际的时区转换 + // 如需时区转换，请使用其他方法 const result = dateTime; return result; }packages/basic/tests/init/utils/date-time/genInitData.spec.js (1)
4-11: 建议在afterAll中清理模拟数据直接修改
typeDefinitionMap可能影响其他测试套件。建议在测试完成后恢复原始状态。♻️ 建议的修复
describe('genInitData function - DateTime handling', () => { + let originalTypeDefinition; + beforeAll(() => { + originalTypeDefinition = typeDefinitionMap['nasl.core.DateTime']; // Mock typeDefinitionMap for testing typeDefinitionMap['nasl.core.DateTime'] = { typeKind: 'primitive', typeName: 'DateTime', typeNamespace: 'nasl.core', }; }); + + afterAll(() => { + if (originalTypeDefinition) { + typeDefinitionMap['nasl.core.DateTime'] = originalTypeDefinition; + } else { + delete typeDefinitionMap['nasl.core.DateTime']; + } + });packages/basic/tests/init/utils/date-time/toString.spec.js (2)
1-1: 变量名遮蔽了全局toString静态分析工具标记：导入的
toString遮蔽了全局toString属性。建议使用别名导入以提高代码清晰度。♻️ 建议的修复
-const { toString, typeDefinitionMap } = require('../../../../src/init/dataTypes/tools'); +const { toString: naslToString, typeDefinitionMap } = require('../../../../src/init/dataTypes/tools');然后在测试中使用
naslToString替代toString。
4-13: 建议在测试后清理模拟数据与
genInitData.spec.js相同，直接修改typeDefinitionMap可能影响其他测试。packages/basic/tests/init/utils/date-time/fromString.spec.js (1)
4-13: 建议在测试后清理模拟数据与其他测试文件相同，直接修改
typeDefinitionMap可能影响其他测试套件。
YufJi
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/basic/src/sdk/modules/utils.ts (2)
306-357: 🛠️ Refactor suggestion | 🟠 MajorJsonSerialize 中
!tz和tz === 'UTC'两个分支逻辑完全相同，存在重复代码。Lines 312-322 和 Lines 324-334 的处理逻辑一模一样（都使用
{ zone: 'UTC' }并格式化为同一模式），可以合并简化：♻️ 建议合并
if (isInputValidNaslDateTime(v)) { - if (!tz) { - let dt = - v instanceof Date - ? DateTime.fromJSDate(v, { zone: 'UTC' }) - : v?.includes('T') - ? DateTime.fromISO(v, { zone: 'UTC' }) - : DateTime.fromFormat(v, 'yyyy-MM-dd HH:mm:ss', { zone: 'UTC' }); - - const d = dt.toFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); - return JSON.stringify(d); - } - if (tz === 'UTC') { - let dt = - v instanceof Date - ? DateTime.fromJSDate(v, { zone: 'UTC' }) - : v?.includes('T') - ? DateTime.fromISO(v, { zone: 'UTC' }) - : DateTime.fromFormat(v, 'yyyy-MM-dd HH:mm:ss', { zone: 'UTC' }); - - const d = dt.toFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); - return JSON.stringify(d); - } - if (tz) { + const targetZone = !tz || tz === 'UTC' ? 'UTC' : getAppTimezone(tz); + { - const targetZone = getAppTimezone(tz); let dt = v instanceof Date ? DateTime.fromJSDate(v, { zone: targetZone }) - : v?.includes('T') + : typeof v === 'string' && v.includes('T') ? DateTime.fromISO(v, { zone: targetZone }) : DateTime.fromFormat(v, 'yyyy-MM-dd HH:mm:ss', { zone: targetZone }); const d = dt.toFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZZ"); return JSON.stringify(d); }同时注意
v?.includes('T')存在与DateFormatter.ts相同的类型安全问题：当v是Date对象以外的非字符串类型时会抛出错误。不过这里v instanceof Date先行判断可以缓解此风险。
1565-1583:⚠️ Potential issue | 🟠 Major
ConvertTimezone函数是一个无操作实现，没有执行任何时区转换。此函数作为公开 API 的一部分被导出，但第 1581 行直接返回未修改的输入值（
const result = dateTime; return result;）。虽然实现了参数校验，但未执行实际的时区转换逻辑。根据代码中的注释，这是来自 v3.1 版本升级脚本的遗留代码，其目的和必要性均有疑问。由于该函数已导出为公开 API，需要：
- 补充 Luxon 库的时区转换实现（参考现有的
addDays、addMonths等实现模式）- 或明确将其标记为已弃用
- 或在无破坏性变更的情况下将其移除
🤖 Fix all issues with AI agents
In `@packages/basic/jest.setup.js`:
- Around line 8-12: Basic.initDataTypes 的 dataTypesMap 目前只注册了
'nasl.core.DateTime'，需要补充对 'nasl.core.Date' 和 'nasl.core.Time' 的注册以保持与库中转换逻辑一致；在
jest.setup.js 中的 Basic.initDataTypes 调用里（dataTypesMap）添加条目 'nasl.core.Date' 和
'nasl.core.Time'，每个条目与现有 'nasl.core.DateTime' 的结构一致（即提供 typeName 字段），确保与
packages/basic/src/init/dataTypes/tools.ts、helper.ts 和测试用例使用的类型名称匹配。
In `@packages/basic/src/sdk/Formatters/DateFormatter.ts`:
- Around line 56-63: The current branching in DateFormatter (the dt assignment
using DateTime.fromISO/fromJSDate) calls value?.includes('T') which throws when
value is a number; change the check to only call includes when value is a string
(e.g., use typeof value === 'string' && value.includes('T')) so numeric
timestamps fall through to DateTime.fromJSDate(new Date(value)); update the dt
assignment that uses DateTime.fromISO and DateTime.fromJSDate accordingly to
handle Date, string-ISO, and numeric inputs safely.
In `@packages/basic/src/sdk/helper.ts`:
- Around line 76-88: The getAppTimezone function's parameter tz is typed as
string but callers pass an optional/undefined value; update the signature of
getAppTimezone to accept undefined (e.g., tz?: string or tz: string | undefined)
so the code and TypeScript reflect that tz can be missing, and ensure any
exported type declarations involving getAppTimezone are updated; check related
callers such as GetDateCount and ToString in utils.ts to confirm their optional
tz? usage aligns with the new signature.
In `@packages/basic/src/sdk/modules/utils.ts`:
- Around line 66-112: The differenceIn* functions (differenceInYears,
differenceInQuarters, differenceInMonths, differenceInWeeks, differenceInDays,
differenceInHours, differenceInMinutes, differenceInSeconds) currently use
Math.floor which incorrectly rounds negative diffs toward -Infinity; replace
Math.floor(...) with Math.trunc(...) in each function so differences are
truncated toward zero (matching date-fns behavior) while keeping the rest of the
DateTime.fromJSDate and .diff(...) logic unchanged.
- Around line 2-3: Remove the leftover date-fns import (getWeekOfMonth) and
replace its use in GetDateCountOld with a Luxon-based calculation: create a
small helper that uses DateTime.fromJSDate(...) (or
DateTime.fromISO/DateTime.fromMillis as appropriate), get startOfMonth.weekday
and the target date.day, then compute weekOfMonth = Math.ceil((dateDay +
startOfMonth.weekday - 1) / 7); update GetDateCountOld to call this helper and
delete the getWeekOfMonth import.
In `@packages/basic/tests/init/utils/date-time/compact.spec.js`:
- Around line 19-22: The test uses new Date(datetime) with datetime =
'2026-02-05 12:00:00' which is implementation-defined and assumes a TZ (e.g.,
comment on isoDatetime expects '2026-02-05T04:00:00.000Z'); fix by making date
parsing deterministic: replace the ambiguous construction around
datetime/jsDate/isoDatetime with an explicit UTC or offset-aware creation (e.g.,
parse '2026-02-05T12:00:00Z' or construct via Date.UTC) or ensure the test sets
TZ to a fixed zone (e.g., TZ=Asia/Shanghai) in the test setup so isoDatetime is
stable across environments. Ensure references to datetime, jsDate and
isoDatetime are updated accordingly.
- Line 1: 测试文件 compact.spec.js 中的 require('./old-basic') 报找不到模块，导致 CI
失败；请在同目录新增模块文件 named old-basic.js（或创建 old-basic/index.js）并导出预期的符号（确保导出能被 const
OldBasic = require('./old-basic') 正常使用），或者修正 compact.spec.js
中的导入路径以指向正确的文件/包；确认文件名大小写与仓库一致并提交到 PR。
- Around line 58-82: The CurrDate/CurrTime tests are flaky due to time advancing
between OldUtils.CurrDate()/Utils.CurrDate() (and CurrTime) calls; update the
tests to freeze system time using Jest fake timers (e.g., jest.useFakeTimers +
jest.setSystemTime) before calling OldUtils.CurrDate/Utils.CurrDate and
OldUtils.CurrTime/Utils.CurrTime, assert equality, then restore real timers with
jest.useRealTimers()/jest.clearAllTimers; ensure both timezone and non-timezone
branches use the frozen timestamp so CurrDate, CurrTime, OldUtils.CurrDate, and
OldUtils.CurrTime return deterministic, matching values.
In `@packages/basic/tests/init/utils/date-time/old-basic/package.json`:
- Around line 1-2: Update the duplicated "name" field in the test fixture
package.json so it no longer conflicts with the root package.json: open the
package.json containing the "name": "@lcap/basic-template" entry and change it
to a unique value (e.g., "@lcap/basic-template-old-fixture") or alternatively
add this fixture directory to Jest's exclusions by updating Jest config
(modulePathIgnorePatterns) to ignore the fixture folder; ensure you only modify
the fixture package.json "name" or Jest config and keep the root package.json
unchanged.
In `@packages/basic/tests/init/utils/date-time/old-basic/README.md`:
- Around line 93-131: The fenced code blocks in README.md that show the
directory structure are missing a language identifier (markdownlint MD040);
update the opening backticks for those blocks (the directory-structure block and
the other fenced block mentioned) from ``` to ```text or ```plaintext so the
blocks declare a language (e.g., change ``` to ```text for the src/ tree block
and the similar block at the other location).
In `@packages/basic/tests/sdk/utils-date-fns-migration.spec.js`:
- Around line 240-246: The test for GetSpecificDaysOfWeek is non-deterministic
because it assumes a +08:00 local timezone; make the test deterministic by
setting the process TZ for the test or computing the expected strings in the
same zone: either (A) set process.env.TZ = 'Asia/Shanghai' (or export TZ) at the
top of packages/basic/tests/sdk/utils-date-fns-migration.spec.js before running
the test, or (B) compute the expected values using the same zone used by the
function (e.g., use DateTime.fromJSDate(date).setZone('America/New_York' or
'Asia/Shanghai').toFormat(...) inside the test) so the expected offsets match
regardless of runner environment; reference the GetSpecificDaysOfWeek call and
update the test accordingly.
🧹 Nitpick comments (14)
packages/basic/tests/init/utils/list/list-arith-functions.spec.js (1)
5-21: 预已存在的问题：.toBeNull缺少括号，断言实际未生效第 7、11、15、19 行的
expect(...).toBeNull缺少()，这只是访问了属性而非调用 matcher，因此这些断言实际上不会执行任何检查。虽然这不是本次 PR 引入的问题，但既然已经在修改此文件，建议一并修复。♻️ 建议修复
fns.forEach((fn) => { - expect(fn.call(utils, undefined)).toBeNull; + expect(fn.call(utils, undefined)).toBeNull(); }); fns.forEach((fn) => { - expect(fn.call(utils, null)).toBeNull; + expect(fn.call(utils, null)).toBeNull(); }); fns.forEach((fn) => { - expect(fn.call(utils, [])).toBeNull; + expect(fn.call(utils, [])).toBeNull(); }); fns.forEach((fn) => { - expect(fn.call(utils, [undefined, null, null])).toBeNull; + expect(fn.call(utils, [undefined, null, null])).toBeNull(); });packages/basic/tests/init/utils/list/list-abnormal-input.spec.js (1)
38-63: 预存问题：.toBeNull和.toThrow缺少括号，断言实际未执行。这些行中
expect(...).toBeNull和expect(() => ...).toThrow只是访问了属性，并没有真正调用 Jest 的 matcher。正确写法应为.toBeNull()和.toThrow()。当前这些断言永远不会失败，等于没有测试。虽然这不是本 PR 引入的问题，但既然在改动此文件，建议顺手修复。
♻️ 建议修复（示例，其他类似行同理）
try { - expect(fn(undefined)).toBeNull; + expect(fn(undefined)).toBeNull(); } catch (err) { - expect(() => fn(undefined)).toThrow; + expect(() => fn(undefined)).toThrow(); }try { - expect(fn(null)).toBeNull; + expect(fn(null)).toBeNull(); } catch (err) { - expect(() => fn(null)).toThrow; + expect(() => fn(null)).toThrow(); }try { - expect(fn([])).toBeNull; + expect(fn([])).toBeNull(); } catch (err) { - expect(() => fn([])).toThrow; + expect(() => fn([])).toThrow(); }同样适用于第 74 行的
.toBeNull。packages/basic/tests/init/utils/date-diff.spec.js (1)
8-18: 三个测试用例使用了相同的名称'DateDiff 按日'。虽然这是已有问题而非本次变更引入，但重复的测试名称会导致测试报告难以区分失败用例。建议后续区分命名，例如
'DateDiff 按日 - 跨天不足一天'、'DateDiff 按日 - 完整天数'等。packages/basic/tests/init/utils/convert.spec.js (1)
58-78: 被注释的Convert到Date/Time类型的测试用例。这些测试在 Luxon 迁移后是否仍然需要？如果
Date和Time类型转换已在新实现中支持，建议取消注释并更新断言；如果已废弃，建议清理删除以避免代码腐化。packages/basic/tests/init/utils/date-time/old-basic/README.md (1)
1-5: 文档放置位置不寻常。这是一份项目级的技术说明文档，但放在了
tests/init/utils/date-time/old-basic/目录下。如果是面向整个@lcap/basic-template包的文档，建议放到packages/basic/根目录或docs/目录下；如果仅用于说明old-basic测试的历史上下文，建议在标题中明确标注范围。packages/basic/tests/sdk/utils-luxon.spec.js (1)
1-7: Utils 变量获取方式与其他测试文件不一致。其他测试文件（如
split.spec.js、convert.spec.js等）均使用const utils = global.Utils;在模块顶层直接赋值，而此文件使用let Utils+beforeAll赋值。建议统一风格。packages/basic/tests/sdk/utils-date-fns-migration.spec.js (3)
268-274: 未使用的变量monday和tuesday。Lines 269-270 声明了
monday和tuesday但从未在断言中使用，属于死代码。♻️ 建议清理
test('isMonday', () => { - const monday = new Date('2024-01-01'); // Monday - const tuesday = new Date('2024-01-02'); - // Note: isMonday 等函数在内部使用，这里通过 GetSpecificDaysOfWeek 间接测试 const result = utils.GetSpecificDaysOfWeek('2024-01-01', '2024-01-07', [1]); // Monday expect(result.length).toBe(1); });
340-346: 空断言expect(true).toBe(true)无实际校验作用。如果目的是验证性能（循环 100 次不崩溃），可以改为验证返回值一致性，或至少添加耗时断言使测试更有意义。
♻️ 建议改进
test('should handle multiple DateDiff calculations', () => { + const start = Date.now(); for (let i = 0; i < 100; i++) { utils.DateDiff('2020-01-01', '2024-12-31', 'd'); } - // If this test passes, performance is acceptable - expect(true).toBe(true); + const elapsed = Date.now() - start; + expect(elapsed).toBeLessThan(1000); });
309-315: 无效日期测试断言过于宽松。注释说明了 Luxon 与 date-fns 的行为差异，但仅验证
typeof result === 'number'不够充分。建议至少断言返回值不为NaN，以确认函数确实产生了有意义的结果。const result = utils.DateDiff('invalid', '2024-01-01', 'd'); - expect(typeof result).toBe('number'); + expect(typeof result).toBe('number'); + expect(Number.isNaN(result)).toBe(false);packages/basic/src/sdk/helper.ts (1)
4-5:@TODO注释已过时。
DateFormatter.ts第 5 行仍然有@TODO: use moment or some other library的注释，但该 PR 的目标正是用 Luxon 替换 moment。虽然该注释不在本文件中，但既然 helper.ts 已完成迁移，建议一并清理。（参见packages/basic/src/sdk/Formatters/DateFormatter.ts第 5 行）packages/basic/src/sdk/Formatters/DateFormatter.ts (1)
4-6:@TODO: use moment or some other library注释已过时。Luxon 已经在本 PR 中引入使用，此 TODO 应删除。
-/** - * `@TODO`: use moment or some other library - */packages/basic/src/sdk/modules/utils.ts (1)
1155-1157:startOf('month')的结果经过多层转换，可以简化。
new Date(DateTime.fromJSDate(date).startOf('month').toFormat('yyyy-MM-dd HH:mm:ss'))链路过长：Luxon → 格式化字符串 → Date 构造。可直接使用.toJSDate()：-const startOfMonth = new Date(DateTime.fromJSDate(date).startOf('month').toFormat('yyyy-MM-dd HH:mm:ss')); +const startOfMonth = DateTime.fromJSDate(date).startOf('month').toJSDate();packages/basic/tests/init/utils/date-time/compact.spec.js (2)
282-304:DateDiff测试中，datetime2等常量可提到循环外。
datetime2、jsDate2、isoDatetime2不依赖calcType，可以移到for循环外部避免重复创建。建议的修复
test('DateDiff', () => { const calcTypes = ['y', 'q', 'M', 'w', 'd', 'h']; + const datetime2 = '2026-02-10 12:00:00'; + const jsDate2 = new Date(datetime2); + const isoDatetime2 = jsDate2.toJSON(); for (const calcType of calcTypes) { - const datetime2 = '2026-02-10 12:00:00'; - const jsDate2 = new Date(datetime2); - const isoDatetime2 = jsDate2.toJSON();
232-233: 跳过的测试用例（GetSpecificDaysOfWeek、FormatTime、genInitData）缺少说明。建议添加注释或 TODO 说明跳过原因和后续计划，方便后续跟进。例如：
// TODO: GetSpecificDaysOfWeek 新旧实现行为不一致，待确认后补充 test.skip('GetSpecificDaysOfWeek', () => {});Also applies to: 257-258, 361-362
4 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/basic/src/sdk/modules/utils/index.ts (1)
1430-1448:⚠️ Potential issue | 🟠 Major
ConvertTimezone返回值类型不匹配，行为已变为 noop该函数类型定义声明返回
string（见packages/vue2/source/types/global.d.ts:338和packages/vue3/source/types/global.d.ts:338），但实现现在直接返回原始输入值而不经过格式化。从测试文件中的旧实现可以看出，原来的行为是进行时区转换并格式化输出（yyyy-MM-dd'T'HH:mm:ssxxx格式）。虽然在代码库中未发现活跃的调用点，但这造成了 API 契约破裂：调用方基于类型声明期望得到格式化的字符串，而现在收到的是未转换的原始值。
需要：同步更新类型定义和实现，或者添加
@deprecated标记并提供替代方案。
🤖 Fix all issues with AI agents
In `@packages/basic/src/sdk/modules/utils/date.ts`:
- Around line 103-105: getWeek and startOfWeek accept an options.weekStartsOn
parameter but ignore it (Luxon DateTime.weekNumber is ISO-based), which is
misleading and will produce wrong results if callers pass weekStartsOn: 0;
update both functions (getWeek and startOfWeek) to validate
options.weekStartsOn: if undefined or 1 proceed as today; if any value !== 1
either throw a clear error or log a warning and document behavior; alternatively
remove the options parameter from both signatures and update callers to avoid
suggesting configurable week start—ensure the chosen approach is applied
consistently and tests/call sites are updated accordingly.
- Around line 43-89: The differenceIn* functions (differenceInYears,
differenceInQuarters, differenceInMonths, differenceInWeeks, differenceInDays,
differenceInHours, differenceInMinutes, differenceInSeconds) use Math.floor
which incorrectly rounds negative differences toward -Infinity; replace
Math.floor(...) with Math.trunc(...) in each function so negative diffs are
truncated toward zero and match date-fns behavior (preserve existing
DateTime.fromJSDate and .diff(...) usage).
In `@packages/basic/src/sdk/modules/utils/index.ts`:
- Around line 1020-1023: The code constructs startOfMonth by formatting a Luxon
DateTime to a string and passing it to new Date(), which yields engine-dependent
parsing; change that to use Luxon's native toJSDate() directly (replace the new
Date(DateTime.fromJSDate(date).startOf('month').toFormat(...)) pattern with
DateTime.fromJSDate(date).startOf('month').toJSDate()) so startOfMonth is a real
Date without relying on string parsing; update any similar uses in this module
(e.g., where startOfMonth is computed) to use toJSDate().
- Around line 178-212: Merge the duplicate branches in JsonSerialize by treating
both !tz and tz === 'UTC' as the same UTC path: compute a zone variable (e.g.,
zone = tz && tz !== 'UTC' ? getAppTimezone(tz) : 'UTC') and use it for DateTime
creation so the three cases collapse to two (UTC and non-UTC). After creating
the DateTime (using DateTime.fromJSDate, fromISO, or fromFormat as now), check
dt.isValid and handle invalid parses explicitly (e.g., return a stable value or
throw/log an error instead of letting toFormat produce "Invalid DateTime");
reference the JsonSerialize function and getAppTimezone to locate the change.
🧹 Nitpick comments (2)
packages/basic/src/sdk/modules/utils/date.ts (1)
3-4: 保留了 date-fns 依赖
getWeekOfMonth仍从date-fnsre-export，说明 date-fns 仍然是运行时依赖。如果迁移目标是完全移除 date-fns，这一项需要后续处理。注释中已表明了这一点，建议添加一个 TODO 标注以便追踪。packages/basic/src/sdk/modules/utils/index.ts (1)
911-922:AddDays/AddMonths/SubDays使用toValue.call(this, ...)传递上下文查看
helper.ts中的toValue实现，当typeKey === 'json'时会调用this.JsonSerialize(date)，因此需要this指向 Utils 实例。使用.call(this)确保了这一点，逻辑正确。但这种隐式耦合较脆弱——
toValue是一个外部 helper 函数，却通过this回调 Utils 实例方法。如果后续toValue被重构或单独使用，容易出错。可以考虑后续将toValue改为显式接收回调参数。
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/basic/src/sdk/modules/utils/date.ts`:
- Around line 121-133: eachDayOfInterval currently returns an empty array when
interval.start > interval.end, which differs from date-fns behavior; update the
function (eachDayOfInterval) to validate the interval after converting to
DateTime (DateTime.fromJSDate) and throw a RangeError (e.g., "Invalid interval")
when start > end so callers get the same error behavior as date-fns; keep the
rest of the day-iteration logic unchanged.
🧹 Nitpick comments (1)
packages/basic/src/sdk/modules/utils/date.ts (1)
3-4: 确认 tree-shaking 是否有效生效，或考虑用 Luxon 自行实现getWeekOfMonth。PR 目标是完全迁移到 Luxon 减小体积，但此处仍 re-export 了
getWeekOfMonth来自 date-fns（在 utils/index.ts 第 939 行被实际使用）。若打包工具 tree-shaking 失效，date-fns 2.30.0 仍会被完整打入 bundle，削弱迁移收益。建议：
- 确认最终产物中 date-fns 的实际体积占比；
- 若影响较大，可用 Luxon DateTime 的
day和week方法自行实现getWeekOfMonth（逻辑并不复杂）。
YufJi
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/basic/src/sdk/modules/utils/index.ts (1)
1420-1433:⚠️ Potential issue | 🟠 Major
ConvertTimezone函数已被彻底阉割，返回输入原值（无操作）该函数原本使用
OA()进行实际的时区转换（见旧代码 old-basic/index.js 第 20594 行），现已变为空操作：const result = dateTime; return result;。虽然保留了输入参数校验，但核心转换逻辑被完全移除。同时存在两个问题：
- 类型签名不符：global.d.ts 声明返回类型为
string，但实现返回的是原始输入值- 公开 API 行为变化：该函数是全局可用的公开接口，任何依赖它进行时区转换的应用代码会在升级后静默获得未转换的结果
建议选择以下方案之一：
- 使用现有的
convertJSDateInTargetTimeZone辅助函数恢复实现（其他日期函数都在使用）- 如果确认该函数不再需要，应添加
@deprecated标记并明确通知用户停用
🤖 Fix all issues with AI agents
In `@packages/basic/src/sdk/modules/utils/index.ts`:
- Around line 1069-1072: Replace Luxon format tokens that use "ZZ" with "ZZZ" in
the four places to maintain moment.js-compatible timezone offset without a
colon: change the format string "yyyy-MM-dd'T'HH:mm:ss.SSSZZ" to use "ZZZ" in
the JsonSerialize, AlterDateTime, GetSpecificDaysOfWeek, and Convert
implementations so they output offsets like +0530; update those format calls
accordingly (or confirm downstream accepts coloned offsets before changing).
🧹 Nitpick comments (3)
packages/basic/src/sdk/modules/utils/index.ts (3)
887-907: 标记废弃合理，建议补充迁移指引。
@deprecated注解是好做法。建议在 JSDoc 中补充@see或替代方案说明（例如推荐使用AlterDateTime），方便下游开发者迁移。
933-965:switch (metric1)各case缺少break，存在 fall-through 风险这是既有问题而非本 PR 引入的，但迁移过程中值得注意：当
metric2不匹配内层任何case时，外层case 'day'会穿透到case 'week'，依此类推，可能产生意外返回值。建议在后续迭代中补充break或改写为if-else。Also applies to: 991-1035
1038-1072:AlterDateTime中addDate在unit不匹配时为undefined如果
unit参数不在已处理的枚举值中，addDate保持undefined，后续format(addDate, ...)调用会导致DateTime.fromJSDate(undefined)返回 invalid DateTime，最终输出"Invalid DateTime"字符串。这是既有问题，但迁移后表现可能有差异（date-fns 对无效输入的容错方式不同）。建议在 switch 后增加兜底处理
case 'year': addDate = addYears(date, amount); break; + default: + return dateString; }
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/basic/src/sdk/helper.ts`:
- Around line 76-88: getAppTimezone may return undefined when
window?.appInfo?.appTimeZone is not set and tz === 'global'; update
getAppTimezone so that globalTimeZone is validated and, if falsy, falls back to
userTimeZone before returning. Concretely, in getAppTimezone compute
userTimeZone via Intl.DateTimeFormat().resolvedOptions().timeZone, compute
globalTimeZone from window?.appInfo?.appTimeZone, then when tz === 'global'
return globalTimeZone || userTimeZone; keep existing behavior for 'user' and
default cases.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/basic/src/sdk/modules/utils/date.ts`:
- Around line 16-17: The comment says the week starts on Monday but the default
weekStartsOn is 0 (Sunday); update the default to match the comment by changing
the default to 1 (Monday) in getWeekOfMonth (and likewise for the other
occurrences in this file that initialize weekStartsOn), or alternatively update
the comment to reflect Sunday—choose one consistent approach and apply it to all
instances that set weekStartsOn so the comment and default value match.
In `@packages/basic/tests/init/utils/date-time/date.spec.js`:
- Around line 22-25: The test uses new Date(datetime) where datetime is a
non‑ISO string with a space separator; update the test to use a deterministic
ISO 8601 string or build the Date explicitly: locate the loop using dates, the
variables jsDate and isoDatetime, and either replace the input datetime with an
ISO string (e.g. '2026-02-05T04:00:00.000Z') or transform the string before
parsing (e.g. datetime.replace(' ', 'T')) or parse components and call new
Date(year, month-1, day, hour, minute, second) so parsing is consistent across
environments; also update the comment for isoDatetime to reflect the actual
expected value.
- Around line 11-20: Remove the duplicate entry in the dates array (the repeated
'2023-01-01 00:00:00' in the dates constant) so each test date is unique, and
remove the unused import of format (the unused symbol format) from the test file
to eliminate the lint warning; update the dates array definition and delete the
import statement referencing format.
🧹 Nitpick comments (1)
packages/basic/src/sdk/modules/utils/date.ts (1)
145-168:getWeekYear使用原生date.getFullYear()和Date构造，与模块其余部分使用 Luxon 的风格不一致。虽然功能上正确，但混用原生
Date操作和 Luxon 会降低可维护性。考虑后续统一为 Luxon API。这不阻塞合并。
This file contains hidden or 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
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.
背景：moment + moment-timezone + date-fns的依赖导致基础库体积过大
解决方式：luxon
Summary by CodeRabbit
新功能
依赖更新
改进
测试