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.

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

Downgrade to using nanoid@3 to support its use in CJS module bundle #307

Merged
merged 2 commits into from
May 1, 2024

Conversation

VeskeR
Copy link
Contributor

@VeskeR VeskeR commented Apr 30, 2024

Resolves #306

Starting from nanoid v4, it only works with ESM projects. We need to support both CJS and ESM bundles for the spaces SDK, so we have the following options to use the nanoid package:

  1. Replace require('nanoid') imports with async await import('nanoid'). This solution is not suitable as the methods we currently use nanoid in are not async, and changing them to async will create cascading effects for a bunch of other methods and change the public API for users.
  2. Replace the nanoid package with an in-project utility function to generate ids. This may be undesired since nanoid uses different crypto packages for its browser and Node.js bundles with some other platform-specific optimizations. Including them locally would not be a trivial change.
  3. Downgrade to using nanoid v3, which supports both CJS and ESM and is still being supported by the developer. This is the option implemented in this PR.

This PR also disables major version updates for nanoid package by GitHub dependabot.

Starting from nanoid v4, it only works with ESM projects. We need to
support both CJS and ESM bundles for the `spaces` SDK, so we have the
following options to use the nanoid package:

1. Replace `require('nanoid')` imports with async `await import('nanoid')`.
   This solution is not suitable as the methods we currently use nanoid
   in are not async, and changing them to async will create cascading
   effects for a bunch of other methods and change the public API for
   users.
2. Replace the nanoid package with an in-project utility function to
   generate ids. This may be undesired since nanoid uses different
   `crypto` packages for its browser and Node.js bundles with some other
   platform-specific optimizations. Including them locally would not be
   a trivial change.
3. Downgrade to using nanoid v3, which supports both CJS and ESM and is
   still being supported by the developers [1]. This is the option
   implemented in this commit.

Resolves #306

[1] https://github.com/ai/nanoid?tab=readme-ov-file#install
@VeskeR VeskeR requested a review from ttypic April 30, 2024 12:06
@github-actions github-actions bot temporarily deployed to staging/pull/307/typedoc April 30, 2024 12:06 Inactive
Copy link
Collaborator

@ttypic ttypic left a comment

Choose a reason for hiding this comment

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

LGTM

@VeskeR VeskeR merged commit cff95e3 into main May 1, 2024
8 checks passed
@VeskeR VeskeR deleted the 306/fix-commonjs-module branch May 1, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Not able to run React Hooks example for Spaces in NextJS 14 app
2 participants