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

feat(string): adds support for generating ULID #2524

Merged

Conversation

brunocleite
Copy link
Contributor

@brunocleite brunocleite commented Nov 3, 2023

There is another type of unique identifier other than UUID that is called ULID Universally Unique Lexicographically Sortable Identifier.
This type of identifier has the benefit of maintaining a sortable order and is useful as primary keys on databases (mainly NoSQL) to get free date-time sorting out of the box. More information here

ULID is a 26 characters string which the first 10 characters are the timestamp as millis from the epoch and the other 16 are the randomness.
The allowed characters are from Crockford's Base32 which excludes letters I, L, and O to avoid confusion with digits.

ulid(): string {
    return (
      this.fromCharacters('012', 1) +
      this.fromCharacters('0123456789ABCDEFGHJKMNPQRSTVWXYZ', 25)
    );
  }

On the code excerpt above it is using the first character as [012] because that would already generate dates up to year 5314 which is more than enough. To confirm this behavior you can go here and try using 2ZZZZZZZZZWZYF0EJ600G2SZHA as an ULID.

@brunocleite brunocleite requested a review from a team as a code owner November 3, 2023 17:26
@xDivisionByZerox xDivisionByZerox added c: feature Request for new feature p: 1-normal Nothing urgent s: awaiting more info Additional information are requested s: needs decision Needs team/maintainer decision m: string Something is referring to the string module labels Nov 3, 2023
@brunocleite
Copy link
Contributor Author

Update: improved the description

@xDivisionByZerox
Copy link
Member

Thank you for your contribution. Since this PR doesn't have a relatet issue, please understand that we might need some time to decide on the usability of this feature. Thank you for your patience.

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.97%. Comparing base (2f93d9d) to head (c8347cd).
Report is 1 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2524   +/-   ##
=======================================
  Coverage   99.96%   99.97%           
=======================================
  Files        2794     2796    +2     
  Lines      227415   227441   +26     
  Branches      949      956    +7     
=======================================
+ Hits       227342   227377   +35     
+ Misses         73       64    -9     
Files with missing lines Coverage Δ
src/internal/base32.ts 100.00% <100.00%> (ø)
src/internal/date.ts 100.00% <100.00%> (ø)
src/modules/date/index.ts 100.00% <100.00%> (ø)
src/modules/string/index.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@ST-DDT ST-DDT added this to the vFuture milestone Nov 7, 2023
@ST-DDT ST-DDT added s: waiting for user interest Waiting for more users interested in this feature and removed s: awaiting more info Additional information are requested s: needs decision Needs team/maintainer decision labels Nov 7, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Nov 7, 2023

Team Decision

  • We checked the project activity and downloads and while it has some, we are waiting for more interest from the community before adding it to our official API.

The PR description contains a script that can be used as a workaround.

@ST-DDT ST-DDT added the has workaround Workaround provided or linked label Nov 7, 2023
@matthewmayer
Copy link
Contributor

Can we add a matching issue so it can collect upvotes?

@ant1m4tt3r
Copy link

didn't find any issues related to this, but I would like to state my support for this feature!

@Shinigami92
Copy link
Member

On the code excerpt above it is using the first character as [012] because that would already generate dates up to year 5314 which is more than enough.

While that might be true, I would be in favor to build it differently and make use of refDate like we do in other functions 🤔

@matthewmayer
Copy link
Contributor

Added #2648 to allow for collecting upvotes (we normally try to get 10 upvotes to indicate sufficient interest) and further promote discussion

@matthewmayer matthewmayer linked an issue Feb 8, 2024 that may be closed by this pull request
@ST-DDT
Copy link
Member

ST-DDT commented Mar 15, 2024

@brunocleite Please fix the failing tests. Probably using pnpm run preflight.

Copy link

netlify bot commented Mar 18, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit c8347cd
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/6707fb12409fcd0008582495
😎 Deploy Preview https://deploy-preview-2524.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

ST-DDT
ST-DDT previously approved these changes Jul 8, 2024
ST-DDT
ST-DDT previously approved these changes Jul 8, 2024
Shinigami92
Shinigami92 previously approved these changes Jul 10, 2024
@xDivisionByZerox
Copy link
Member

@brunocleite FYI: This PR is ready, but is still waiting for the release of v9.0. After the release there will be a time of about 2 weeks in which we freeze updates on the next branch, in case a critical error appears in v9.0. After this, your PR will be merged to be included in the release of v9.1.

@ejcheng ejcheng added the s: on hold Blocked by something or frozen to avoid conflicts label Aug 24, 2024
@ST-DDT ST-DDT removed the s: on hold Blocked by something or frozen to avoid conflicts label Sep 14, 2024
ejcheng
ejcheng previously approved these changes Sep 23, 2024
@ST-DDT ST-DDT added the needs rebase There is a merge conflict label Sep 26, 2024
@ST-DDT
Copy link
Member

ST-DDT commented Sep 27, 2024

Dear @brunocleite,

Thank you for your patience and contributions so far. We wanted to provide you with a brief update.

We are pleased to inform you that we have addressed all the known bugs in version 9.0. We will now take a short break and resume work around October 7, 2024, to begin merging new features for v9.1, including yours.

In the meantime, we hope you have a great time, and we sincerely appreciate your ongoing support and contributions.

Best regards,
your FakerJS Team


FYI: There is currently a merge conflict, could you please fix until then?

…ulid-generation

# Conflicts:
#	src/modules/date/index.ts
#	test/utils/__snapshots__/base32.spec.ts.snap
@brunocleite
Copy link
Contributor Author

brunocleite commented Sep 27, 2024

Dear @brunocleite,

Thank you for your patience and contributions so far. We wanted to provide you with a brief update.

We are pleased to inform you that we have addressed all the known bugs in version 9.0. We will now take a short break and resume work around October 7, 2024, to begin merging new features for v9.1, including yours.

In the meantime, we hope you have a great time, and we sincerely appreciate your ongoing support and contributions.

Best regards, your FakerJS Team

FYI: There is currently a merge conflict, could you please fix until then?

Thank you for the update. I'm honored to be contributing and will resolve the merge conflicts promptly.

@xDivisionByZerox xDivisionByZerox removed the needs rebase There is a merge conflict label Oct 10, 2024
@Shinigami92 Shinigami92 merged commit 5b1c858 into faker-js:next Oct 10, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature has workaround Workaround provided or linked m: string Something is referring to the string module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for generating ULID
7 participants