Skip to content
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.

Sign up for GitHub

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

Jump to bottom

chore: remove datetime package dependencies to lodash #6751

Merged
merged 2 commits into from
May 21, 2024
Merged

chore: remove datetime package dependencies to lodash #6751

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02235a7
chore: remove datetime package dependencies to lodash
PetrusAsikainen Mar 26, 2024
dc05914
improve consistency with previous impl with invalid args
PetrusAsikainen Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/datetime/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
"classnames": "^2.3.1",
"date-fns": "^2.28.0",
"date-fns-tz": "^2.0.0",
"lodash": "^4.17.21",
"react-day-picker": "7.4.9",
"tslib": "~2.6.2"
},
Expand All @@ -65,7 +64,6 @@
"@blueprintjs/karma-build-scripts": "workspace:^",
"@blueprintjs/node-build-scripts": "workspace:^",
"@blueprintjs/test-commons": "workspace:^",
"@types/lodash": "~4.14.202",
"enzyme": "^3.11.0",
"karma": "^6.4.2",
"mocha": "^10.2.0",
Expand Down
23 changes: 12 additions & 11 deletions packages/datetime/src/common/getTimezone.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,23 @@
* limitations under the License.
*/

import memoize from "lodash/memoize";

import { UTC_TIME } from "./timezoneItems";

/**
* Gets the users current time zone, for example "Europe/Oslo".
* This is currently backed by the browser or computer's locale setting.
* Keep the returned timezone constant for a page load.
*/
export const getCurrentTimezone: () => string = memoize(guessTimezone);
let memoizedTimezone: string | null = null;

/**
* Unsupported in IE, which newer Blueprint components do not support.
* In the unlikely case that the browser API returns undefined, we default to UTC.
*
* @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/resolvedOptions
* Gets the users current time zone, for example "Europe/Oslo".
* This is currently backed by the browser or computer's locale setting.
*/
function guessTimezone(): string {
return Intl.DateTimeFormat().resolvedOptions().timeZone ?? UTC_TIME.ianaCode;
export function getCurrentTimezone(): string {
if (memoizedTimezone === null) {
// Unsupported in IE, which newer Blueprint components do not support.
// In the unlikely case that the browser API returns undefined, we default to UTC.
// @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/DateTimeFormat/resolvedOptions
memoizedTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone ?? UTC_TIME.ianaCode;
}
return memoizedTimezone;
}
3 changes: 1 addition & 2 deletions packages/datetime/src/common/timezoneUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/

import { formatInTimeZone, utcToZonedTime, zonedTimeToUtc } from "date-fns-tz";
import isEmpty from "lodash/isEmpty";

import { getCurrentTimezone } from "./getTimezone";
import { TimePrecision } from "./timePrecision";
Expand Down Expand Up @@ -69,7 +68,7 @@ export function getDateObjectFromIsoString(
if (value === undefined) {
return undefined;
}
if (value === null || isEmpty(value)) {
if (value === null || value === "") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (value === null || value === "") {
if (value === null || value === "" || typeof value !== "string") {

Can you add a typeof check here to be extra defensive in the event a non-TypeScript consumer is passing in an incorrect value here (such as a number) and expecting to get null returned?

isEmpty evaluates to true on numbers, so I'd like to preserve that behavior to avoid an accidental break.

Copy link
Contributor Author

@PetrusAsikainen PetrusAsikainen Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Seems reasonable. Interesting that here lodash.isEmpty very much doesn't do what it says on the tin:

Checks if value is an empty object, collection, map, or set.

...which a number clearly isn't - handling for primitives isn't specified.

return null;
}
const date = new Date(value);
Expand Down
1 change: 0 additions & 1 deletion packages/datetime2/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
"@blueprintjs/karma-build-scripts": "workspace:^",
"@blueprintjs/node-build-scripts": "workspace:^",
"@blueprintjs/test-commons": "workspace:^",
"@types/lodash": "~4.14.202",
"enzyme": "^3.11.0",
"karma": "^6.4.2",
"mocha": "^10.2.0",
Expand Down
3 changes: 0 additions & 3 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,6 @@ __metadata:
"@blueprintjs/karma-build-scripts": "workspace:^"
"@blueprintjs/node-build-scripts": "workspace:^"
"@blueprintjs/test-commons": "workspace:^"
"@types/lodash": "npm:~4.14.202"
classnames: "npm:^2.3.1"
date-fns: "npm:^2.28.0"
enzyme: "npm:^3.11.0"
Expand Down Expand Up @@ -531,13 +530,11 @@ __metadata:
"@blueprintjs/node-build-scripts": "workspace:^"
"@blueprintjs/select": "workspace:^"
"@blueprintjs/test-commons": "workspace:^"
"@types/lodash": "npm:~4.14.202"
classnames: "npm:^2.3.1"
date-fns: "npm:^2.28.0"
date-fns-tz: "npm:^2.0.0"
enzyme: "npm:^3.11.0"
karma: "npm:^6.4.2"
lodash: "npm:^4.17.21"
mocha: "npm:^10.2.0"
npm-run-all: "npm:^4.1.5"
react: "npm:^16.14.0"
Expand Down