formkit / tempo

šŸ“† Parse, format, manipulate, and internationalize dates and times in JavaScript and TypeScript.
https://tempo.formkit.com
MIT License
2.37k stars 33 forks source link

Bugfix deviceTZ undefined case error #33

Closed mori-hisayuki closed 8 months ago

mori-hisayuki commented 8 months ago

Upgrading to version 0.0.14 caused the format function to malfunction. I believe the issue lies within this piece of code.

src/format.ts

  // We need to apply an offset to the date so that it can be formatted as UTC.
  tz ??= deviceTZ()
  if (tz.toLowerCase() !== "utc") {
    inputDateOrOptions = removeOffset(
      inputDateOrOptions,
      offset(inputDateOrOptions, tz, "utc")
    )
  }

The content of deviceTZ is obtained from Intl.DateTimeFormat().resolvedOptions().timeZone, and this function requires the TZ environment variable to be set when executed in the local environment. If TZ is not set, deviceTZ will return undefined, causing tz to also become undefined, which results in an error when attempting to use tz.toLowerCase().

Also, if deviceTZ is undefined, tz needs to be specified in the FormatOptions, which means it can only be used in the FormatOptions pattern.

I think it would be beneficial to include a branching condition for cases where deviceTZ is undefined but tz is not set to UTC.

justin-schroeder commented 8 months ago

Interesting ā€” thanks for the spotlight on this. For my own personal learnings...what environment have you used where no TZ was set? All the test devices ive used had this set, but that is by no means a comprehensive test suite.

mori-hisayuki commented 8 months ago

Thank you for your response!

It sounds like you were working in a Docker container environment for development (VSCode's devcontainer).

the issue was that you didn't specify the TZ environment variable in the Dockerfile. This became problematic when upgrading to version 0.0.14 because the code depended on the TZ environment variable, which wasn't set. This realization occurred because the code was working fine in version 0.0.13, but started failing after the upgrade to 0.0.14 due to the dependency on TZ.

mori-hisayuki commented 8 months ago

If it's necessary to register the TZ environment variable in advance to use tempo, I thought it would be good to write about it in the documentation!