Skip to content

Bugfix deviceTZ undefined case error #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mori-hisayuki opened this issue Mar 6, 2024 · 3 comments · Fixed by #34
Closed

Bugfix deviceTZ undefined case error #33

mori-hisayuki opened this issue Mar 6, 2024 · 3 comments · Fixed by #34

Comments

@mori-hisayuki
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

mori-hisayuki commented Mar 6, 2024

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
Copy link
Contributor Author

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!

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 a pull request may close this issue.

2 participants