Skip to content

fix: remove lodash functions to decrease bundle size #37

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

Merged
merged 4 commits into from
Dec 5, 2022
Merged

fix: remove lodash functions to decrease bundle size #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f16f958
fix: remove lodash functions to decrease bundle size
gismya Nov 11, 2022
2afeac0
Removed last lodash function
gismya Nov 14, 2022
a4c3d17
Added prettierrc and formatted
gismya Nov 14, 2022
ff9a633
Merge branch 'main' into decrease-bundle-size
gismya Dec 5, 2022
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
1 change: 1 addition & 0 deletions .prettierrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
19 changes: 9 additions & 10 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@
"main.d.ts"
],
"devDependencies": {
"@rollup/plugin-commonjs": "^22.0.2",
"@rollup/plugin-commonjs": "^23.0.2",
"cross-fetch": "^3.1.5",
"eslint": "^8.24.0",
"eslint": "^8.27.0",
"eslint-config-react-app": "^7.0.1",
"jsdom": "^20.0.0",
"jsdom": "^20.0.2",
"lint-staged": "^13.0.3",
"msw": "^0.47.3",
"msw": "^0.48.1",
"prettier": "^2.7.1",
"typescript": "^4.8.3",
"vite": "^3.1.3",
"vitest": "^0.23.4"
"typescript": "^4.8.4",
"vite": "^3.2.3",
"vitest": "^0.25.1"
},
"repository": {
"type": "git",
Expand All @@ -51,8 +51,7 @@
},
"homepage": "http://ftrack.com",
"dependencies": {
"lodash": "^4.17.21",
"loglevel": "^1.8.0",
"loglevel": "^1.8.1",
"moment": "^2.29.4",
"uuid": "^9.0.0"
},
Expand All @@ -61,4 +60,4 @@
"*.{js,css,md,json,jsx,scss,yml}": "prettier --write"
},
"packageManager": "yarn@3.2.3"
}
}
38 changes: 18 additions & 20 deletions source/session.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
// :copyright: Copyright (c) 2016 ftrack

import forIn from "lodash/forIn";
import isArray from "lodash/isArray";
import isString from "lodash/isString";
import isPlainObject from "lodash/isPlainObject";
import find from "lodash/find";
import moment from "moment";
import loglevel from "loglevel";
import { v4 as uuidV4 } from "uuid";
Expand Down Expand Up @@ -189,7 +183,7 @@ export class Session {
* @return {Array|null} List of primary key attributes.
*/
getPrimaryKeyAttributes(entityType) {
const schema = find(this.schemas, (item) => item.id === entityType);
const schema = this.schemas.find((item) => item.id === entityType);
if (!schema || !schema.primary_key) {
logger.warn("Primary key could not be found for: ", entityType);
return null;
Expand Down Expand Up @@ -231,9 +225,11 @@ export class Session {

if (data && data.constructor === Object) {
const out = {};
forIn(data, (value, key) => {
out[key] = this.encode(value);
});
for (const key in data) {
if (data.hasOwnProperty(key)) {
out[key] = this.encode(data[key]);
}
}

return out;
}
Expand Down Expand Up @@ -304,15 +300,16 @@ export class Session {
* @param {*} data The data to decode.
* @return {*} Decoded data
*/

decode(data, identityMap = {}) {
if (data == null) {
return data;
Copy link
Contributor

Choose a reason for hiding this comment

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

what happened with this initial null check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it. The rest of the checks are not picking up null, and the "default" return is also data. That, combined with it being loose equality and thus probably subject to some kind of weird quirks that I didn't feel like googling, made me remove it.

Copy link
Contributor

@lucaas lucaas Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Loose equality for null can be used for null or undefined:

> "null == null:	" true
> "undefined == null:	" true
> "0 == null:		" false
> "false == null:	" false

Since undefined is not valid JSON (which is what is being decoded), this will only be null. null fails the checks, and is returned as passed in as before. All good!

} else if (isArray(data)) {
if (Array.isArray(data)) {
return this._decodeArray(data, identityMap);
} else if (isPlainObject(data)) {
}
if (typeof data === "object" && data?.constructor === Object) {
if (data.__entity_type__) {
return this._mergeEntity(data, identityMap);
} else if (data.__type__ === "datetime") {
}
if (data.__type__ === "datetime") {
return this._decodeDateTime(data);
}
return this._decodePlainObject(data, identityMap);
Expand Down Expand Up @@ -383,10 +380,11 @@ export class Session {
// instead of pointing them to the same instance?
const mergedEntity = identityMap[identifier];

forIn(entity, (value, key) => {
mergedEntity[key] = this.decode(value, identityMap);
});

for (const key in entity) {
if (entity.hasOwnProperty(key)) {
mergedEntity[key] = this.decode(entity[key], identityMap);
}
}
return mergedEntity;
}

Expand Down Expand Up @@ -546,7 +544,7 @@ export class Session {
const criteria = keys.map((identifyingKey) => {
let value = data[identifyingKey];

if (isString(value)) {
if (value != null && typeof value.valueOf() === "string") {
value = `"${value}"`;
} else if (value && value._isAMomentObject) {
// Server does not store microsecond or timezone currently so
Expand Down
10 changes: 8 additions & 2 deletions test/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// :copyright: Copyright (c) 2022 ftrack
import { rest } from "msw";
import fs from "fs/promises";
import { pick } from "lodash";
import querySchemas from "./fixtures/query_schemas.json";
import queryServerInformation from "./fixtures/query_server_information.json";
import getUploadMetadata from "./fixtures/get_upload_metadata.json";
Expand All @@ -14,7 +13,14 @@ function authenticate(req) {
}
return true;
}

function pick(object, keys) {
return keys.reduce((obj, key) => {
if (object && object.hasOwnProperty(key)) {
obj[key] = object[key];
}
return obj;
}, {});
}
const handlers = [
rest.post("http://ftrack.test/api", async (req, res, ctx) => {
if (!authenticate(req)) {
Expand Down
2 changes: 1 addition & 1 deletion vite.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module.exports = defineConfig({
rollupOptions: {
// make sure to externalize deps that shouldn't be bundled
// into your library
external: ["moment", "uuid", "lodash", "loglevel"],
external: ["moment", "uuid", "loglevel"],
output: {
globals: {
"ftrack-javascript-api": "ftrack",
Expand Down
Loading