ng-in-viewport icon indicating copy to clipboard operation
ng-in-viewport copied to clipboard

refactor: remove lodash usages and replace with native JS

Open vmasek opened this issue 10 months ago • 5 comments

This PR replaces lodash usages by native JS check and simple UID generator.

We are in effort to remove lodash from our production dependencies and since we use this library, I figured it would be nice to replace it as it does not rely on anything crucial or complex.

vmasek avatar Mar 27 '24 10:03 vmasek

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ng-in-viewport ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 27, 2024 10:34am

vercel[bot] avatar Mar 27 '24 10:03 vercel[bot]

@vmasek is that big deal for you? Having lodash-es as peer dependency? It already ships with similar implementation, but for those who are using lodash it's beneficial to not bundle same code many times as it's tree shakable.

Could you tell my why it's so beneficial for you to drop lodash from your dependency list? I'm not saying no, but you need to convince my with valid reasons.

We ideally should use globalThis.crypto.randomUUID() or switch to https://www.npmjs.com/package/nanoid as an alternative.

k3nsei avatar Apr 03 '24 13:04 k3nsei

Please also add yourself as a contributor here are instructions https://allcontributors.org/docs/en/cli/usage#all-contributors-add

k3nsei avatar Apr 03 '24 14:04 k3nsei

To be clear, we don't mind lodash as peer dep. What is the point is that due to the library relying on some of those helpers in runtime, we get lodash bundled-in in the initial loading bundle. It was easy to replace these runtime checks and add a simple uid generator, so we figured you might be willing to drop it completely. In the end, fewer dependencies make it more lightweight to use this lib in general.

Bundling wise (ESBuild), I am not sure if it is only our use case, but since we hacked around in imports locally, we lowered the initial bundles footprint and traced the lodash inclusion to this lib.

I'll be very happy if you consider the PR.

I can try to add myself to contributors, but I am on mobile only for a few weeks now and and it is not very straightforward 😀

vmasek avatar Apr 03 '24 14:04 vmasek

@vmasek I recommend using this feature from npm https://docs.npmjs.com/cli/v10/configuring-npm/package-json#overrides Personally I think replacing lodash-es for all library users is a bad idea. As it will increase their bundle sizes. However, I will consider it for v18. For your use, you can use overrides in your package.json and use your own replacement. Example provided below. Btw. if you want to restrict usage of lodash in your project internaly you can always use eslint https://eslint.org/docs/latest/rules/no-restricted-imports#importnames

package.json

{
  "dependencies": {
    "lodash-es": "file:./invp-utils",
  },
  "overrides": {
    "ng-in-viewport": {
      "lodash-es": "$lodash-es"
    }
  }
}

invp-utils/package.json

{
  "name": "invp-utils",
  "version": "0.0.0",
  "private": true,
  "exports": {
    "./package.json": {
      "default": "./package.json"
    },
    ".": {
      "esm2022": "./index.mjs",
      "esm": "./index.mjs",
      "default": "./index.mjs"
    }
  },
  "module": "index.mjs"
}

invp-utils/index.mjs

/**
 * @param {unknown} value
 * @returns {value is boolean}
 */
export const isBoolean = (value) => typeof value === 'boolean';

/**
 * @param {unknown} value
 * @returns {value is (...args: any[]) => any}
 */
export const isFunction = (value) => typeof value === 'function';

/**
 * @param {unknown} value
 * @returns {value is null | undefined}
 */
export const isNil = (value) => value == null;

/**
 * @param {unknown} value
 * @returns {value is number}
 */
export const isNumber = (value) => typeof value === 'number';

/**
 * @param {unknown} value
 * @returns {value is string}
 */
export const isString = (value) => typeof value === 'string';

/**
 * @returns {string}
 */
export const uniqueId = () => {
  return globalThis.crypto.randomUUID();
};

k3nsei avatar Apr 27 '24 14:04 k3nsei

@vmasek I will now close this PR and release a new slimmed-down version once Angular v18 is released. I would also extract config strict checks out of production and give a way to load them only in dev mode.

k3nsei avatar May 06 '24 15:05 k3nsei