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.

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

Jump to bottom

fix(store): use lib0 sha256 when api not available #6454

Merged
merged 1 commit into from
Mar 12, 2024
Merged

fix(store): use lib0 sha256 when api not available #6454

merged 1 commit into from
Mar 12, 2024

Conversation

EYHN
Copy link
Member

@EYHN EYHN commented Mar 11, 2024 •
edited
Loading

@vercel Vercel
Copy link

vercel bot commented Mar 11, 2024 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
blocksuite ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 12, 2024 9:43am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
blocksuite-docs ⬜️ Ignored (Inspect) Visit Preview Mar 12, 2024 9:43am

@EYHN EYHN marked this pull request as ready for review March 11, 2024 14:55
@EYHN EYHN mentioned this pull request Mar 11, 2024
Closed
1 task
@darkskygit
Copy link
Member

i don't think it's a good idea to maintain a hash implementation ourself
i think this library is more suitable: https://github.com/Daninet/hash-wasm , what do you think?

@fourdim
Copy link
Contributor

fourdim commented Mar 11, 2024

What about lib0? We have already introduced it as a dependency.
https://github.com/dmonad/lib0/blob/main/hash/sha256.js

doodlewind
doodlewind reviewed Mar 11, 2024
View reviewed changes
Copy link
Member

@doodlewind doodlewind left a comment

Choose a reason for hiding this comment

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

I'd say the idea using lib0 looks pretty nice!

@Saul-Mirone
Copy link
Collaborator

The changes look good to me but I prefer the idea using lib0.

Brooooooklyn
Brooooooklyn requested changes Mar 12, 2024
View reviewed changes
Copy link
Member

@Brooooooklyn Brooooooklyn left a comment

Choose a reason for hiding this comment

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

Let's use the impl in lib0

@EYHN EYHN force-pushed the eyhn/fix/vanilla-sha256 branch from 3f476ab to b25e7a3 Compare March 12, 2024 09:38
@EYHN EYHN changed the title fix(store): use pure js sha256 when api not available fix(store): use lib0 sha256 when api not available Mar 12, 2024
@doodlewind doodlewind merged commit 945e9c4 into master Mar 12, 2024
20 checks passed
@doodlewind doodlewind deleted the eyhn/fix/vanilla-sha256 branch March 12, 2024 10:05
@regischen regischen mentioned this pull request Mar 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darkskygit darkskygit darkskygit approved these changes

@doodlewind doodlewind doodlewind approved these changes

@Brooooooklyn Brooooooklyn Awaiting requested review from Brooooooklyn

@Saul-Mirone Saul-Mirone Awaiting requested review from Saul-Mirone

Assignees
No one assigned
Labels
None yet
Projects
BlockSuite Project
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

​Failed to upload image! crypto.subtle is undefined
6 participants
@EYHN @darkskygit @fourdim @Saul-Mirone @Brooooooklyn @doodlewind