fix(root): Replace shortid with nanoid #7720

SokratisVidros · 2025-02-14T07:00:58Z

What changed? Why was the change needed?

Shortid is deprecated and marked as unsafe. https://www.npmjs.com/package/shortid

See also https://github.com/novuhq/packages-enterprise/pull/267

netlify · 2025-02-14T07:01:29Z

✅ Deploy Preview for dev-web-novu ready!

Name	Link
🔨 Latest commit	`ad585e6`
🔍 Latest deploy log	https://app.netlify.com/sites/dev-web-novu/deploys/67be079a57786b0008d46671
😎 Deploy Preview	https://deploy-preview-7720.dashboard.novu-staging.co
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

pkg-pr-new · 2025-02-14T07:05:10Z

Open in Stackblitz

npm i https://pkg.pr.new/novuhq/novu/@novu/providers@7720

commit: a6472cd

SokratisVidros · 2025-02-14T07:05:17Z

apps/api/src/app/integrations/usecases/create-integration/create-integration.usecase.ts

@@ -149,7 +149,7 @@ export class CreateIntegration {
      const defaultName =
        providers.find((provider) => provider.id === command.providerId)?.displayName ?? providerIdCapitalized;
      const name = command.name ?? defaultName;
-      const identifier = command.identifier ?? `${slugify(name)}-${shortid.generate()}`;
+      const identifier = command.identifier ?? `${slugify(name)}-${nanoid()}`;


Maybe we need a longer id here as the default length is 4.

netlify · 2025-02-14T07:08:49Z

✅ Deploy Preview for dashboard-v2-novu-staging ready!

Name	Link
🔨 Latest commit	`a6472cd`
🔍 Latest deploy log	https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/67cb173e1768e100085a9705
😎 Deploy Preview	https://deploy-preview-7720.dashboard-v2.novu-staging.co
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

djabarovgeorge · 2025-02-16T16:34:36Z

libs/application-generic/package.json

@@ -94,7 +94,6 @@
    "rrule": "^2.7.2",
    "rxjs": "7.8.1",
    "sanitize-html": "^2.4.0",
-    "shortid": "^2.2.16",


djabarovgeorge · 2025-02-16T16:35:18Z

libs/testing/src/integration.service.ts

@@ -62,7 +62,7 @@ export class IntegrationService {
      channel,
      credentials: {},
      active,
-      identifier: `${slugify(name)}-${shortid.generate()}`,
+      identifier: `${slugify(name)}-${nanoid()}`,


Suggested change

identifier: `${slugify(name)}-${nanoid()}`,

identifier: `${slugify(name)}-${shortId()}`,

please reuse the short Id from application-generic, so we will have the same alphabet even in tests

preferably will be to create eslint to prevent imports from nanoid

djabarovgeorge · 2025-02-16T16:35:57Z

apps/api/src/app/integrations/usecases/create-integration/create-integration.usecase.ts

@@ -149,7 +149,7 @@ export class CreateIntegration {
      const defaultName =
        providers.find((provider) => provider.id === command.providerId)?.displayName ?? providerIdCapitalized;
      const name = command.name ?? defaultName;
-      const identifier = command.identifier ?? `${slugify(name)}-${shortid.generate()}`;
+      const identifier = command.identifier ?? `${slugify(name)}-${nanoid()}`;


Suggested change

const identifier = command.identifier ?? `${slugify(name)}-${nanoid()}`;

const identifier = command.identifier ?? `${slugify(name)}-${shortId()}`;

the default is length of 21
FYI for the future workflow and steps short id we had length of 6 #7029

regardless please use shortId from libs/application-generic/src/utils/generate-id.ts so we will have the same alphabet on our identifiers.

djabarovgeorge

Looks good to me, we just need to update the last place to use the internal shortId func

SokratisVidros · 2025-02-25T18:10:11Z

@djabarovgeorge All comments have been addressed. Please take another look.

djabarovgeorge · 2025-03-03T09:34:10Z

apps/api/src/app/integrations/usecases/create-integration/create-integration.usecase.ts

@@ -149,7 +149,7 @@ export class CreateIntegration {
      const defaultName =
        providers.find((provider) => provider.id === command.providerId)?.displayName ?? providerIdCapitalized;
      const name = command.name ?? defaultName;
-      const identifier = command.identifier ?? `${slugify(name)}-${shortid.generate()}`;
+      const identifier = command.identifier ?? `${slugify(name)}-${shortId()}`;


before, we used shortId, which generated ids 7+ characters long

do we want to increase the length here as well?
reasons:

to align with old ids.

for future multi-tenancy support, which may require a larger identifier.

djabarovgeorge

looks good, please check the last 2 comments that left.

Shortid is deprecated and marked as unsafe

SokratisVidros requested review from LetItRock, merrcury and djabarovgeorge February 14, 2025 07:00

SokratisVidros self-assigned this Feb 14, 2025

SokratisVidros had a problem deploying to Linting February 14, 2025 07:01 — with GitHub Actions Failure

SokratisVidros temporarily deployed to Linting February 14, 2025 07:01 — with GitHub Actions Inactive

github-actions bot added @novu/worker @novu/dashboard @novu/api-service labels Feb 14, 2025

SokratisVidros had a problem deploying to Linting February 14, 2025 07:02 — with GitHub Actions Failure

SokratisVidros temporarily deployed to Linting February 14, 2025 07:02 — with GitHub Actions Inactive

SokratisVidros commented Feb 14, 2025

View reviewed changes

github-actions bot removed the @novu/dashboard label Feb 14, 2025

SokratisVidros temporarily deployed to Linting February 14, 2025 07:06 — with GitHub Actions Inactive

SokratisVidros had a problem deploying to Linting February 14, 2025 07:06 — with GitHub Actions Failure

SokratisVidros temporarily deployed to Linting February 14, 2025 07:06 — with GitHub Actions Inactive

merrcury approved these changes Feb 14, 2025

View reviewed changes

djabarovgeorge reviewed Feb 16, 2025

View reviewed changes

djabarovgeorge requested changes Feb 16, 2025

View reviewed changes

SokratisVidros force-pushed the remove_shortid branch from 59a7874 to 3cc8299 Compare February 25, 2025 18:07

SokratisVidros had a problem deploying to Linting February 25, 2025 18:07 — with GitHub Actions Failure

SokratisVidros temporarily deployed to Linting February 25, 2025 18:07 — with GitHub Actions Inactive

SokratisVidros force-pushed the remove_shortid branch from 3cc8299 to ad585e6 Compare February 25, 2025 18:10

SokratisVidros had a problem deploying to Linting February 25, 2025 18:10 — with GitHub Actions Failure

SokratisVidros temporarily deployed to Linting February 25, 2025 18:10 — with GitHub Actions Inactive

djabarovgeorge reviewed Mar 3, 2025

View reviewed changes

djabarovgeorge approved these changes Mar 3, 2025

View reviewed changes

SokratisVidros added 2 commits March 7, 2025 17:49

fix(root): Replace shortid with nanoid

1353342

Shortid is deprecated and marked as unsafe

fixup! fix(root): Replace shortid with nanoid

a6472cd

SokratisVidros force-pushed the remove_shortid branch from ad585e6 to a6472cd Compare March 7, 2025 15:56

SokratisVidros temporarily deployed to Linting March 7, 2025 15:56 — with GitHub Actions Inactive

SokratisVidros had a problem deploying to Linting March 7, 2025 15:56 — with GitHub Actions Failure

SokratisVidros temporarily deployed to Linting March 7, 2025 15:56 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(root): Replace shortid with nanoid #7720

fix(root): Replace shortid with nanoid #7720

SokratisVidros commented Feb 14, 2025

netlify bot commented Feb 14, 2025 •

edited

Loading

pkg-pr-new bot commented Feb 14, 2025 •

edited

Loading

SokratisVidros Feb 14, 2025

netlify bot commented Feb 14, 2025 •

edited

Loading

djabarovgeorge Feb 16, 2025

djabarovgeorge Feb 16, 2025 •

edited

Loading

djabarovgeorge Mar 3, 2025 •

edited

Loading

djabarovgeorge Mar 3, 2025

djabarovgeorge Feb 16, 2025 •

edited

Loading

djabarovgeorge left a comment

SokratisVidros commented Feb 25, 2025

djabarovgeorge Mar 3, 2025

djabarovgeorge left a comment

	identifier: `${slugify(name)}-${nanoid()}`,
	identifier: `${slugify(name)}-${shortId()}`,

	const identifier = command.identifier ?? `${slugify(name)}-${nanoid()}`;
	const identifier = command.identifier ?? `${slugify(name)}-${shortId()}`;

fix(root): Replace shortid with nanoid #7720

Are you sure you want to change the base?

fix(root): Replace shortid with nanoid #7720

Conversation

SokratisVidros commented Feb 14, 2025

What changed? Why was the change needed?

netlify bot commented Feb 14, 2025 • edited Loading

✅ Deploy Preview for dev-web-novu ready!

pkg-pr-new bot commented Feb 14, 2025 • edited Loading

SokratisVidros Feb 14, 2025

Choose a reason for hiding this comment

netlify bot commented Feb 14, 2025 • edited Loading

✅ Deploy Preview for dashboard-v2-novu-staging ready!

djabarovgeorge Feb 16, 2025

Choose a reason for hiding this comment

djabarovgeorge Feb 16, 2025 • edited Loading

Choose a reason for hiding this comment

djabarovgeorge Mar 3, 2025 • edited Loading

Choose a reason for hiding this comment

djabarovgeorge Mar 3, 2025

Choose a reason for hiding this comment

djabarovgeorge Feb 16, 2025 • edited Loading

Choose a reason for hiding this comment

djabarovgeorge left a comment

Choose a reason for hiding this comment

SokratisVidros commented Feb 25, 2025

djabarovgeorge Mar 3, 2025

Choose a reason for hiding this comment

djabarovgeorge left a comment

Choose a reason for hiding this comment

netlify bot commented Feb 14, 2025 •

edited

Loading

pkg-pr-new bot commented Feb 14, 2025 •

edited

Loading

netlify bot commented Feb 14, 2025 •

edited

Loading

djabarovgeorge Feb 16, 2025 •

edited

Loading

djabarovgeorge Mar 3, 2025 •

edited

Loading

djabarovgeorge Feb 16, 2025 •

edited

Loading