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(package): add dictionary compression #276

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

mimi89999
Copy link
Contributor

Even when including fflate in the bundle, I still got a compression level of 1.6

Copy link
Collaborator

@MrWook MrWook left a comment

Choose a reason for hiding this comment

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

Hey, thank you very much for the beginning of the compression!

Maybe you could implement a first implementation from lifthrasiir in #254
This should reduce the dictionary even more 🤔 Or we do it in another PR which is totally fine too!

@@ -1,3 +1,6 @@
import { compressSync, strToU8 } from 'fflate'
import { encodeBase85 } from '@alttiri/base85'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really like the idea of using a dependency with almost no downloads. It's kind of a security concern. Maybe we should copy it into this repository instead of depending on it.
I need to think about it 🤔

const data = Array.isArray(parsed)
? `"${parsed.join(',')}".split(',')`
: JSON.stringify(parsed)
var code
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lint rules, no var allowed. This need to be a let

var code
if (Array.isArray(parsed)) {
const data = parsed.join(',')
const data_buf = strToU8(data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lint rule: Need to be camelCase

@MrWook MrWook added the enhancement New feature or request label Sep 27, 2024
@mimi89999
Copy link
Contributor Author

Hey, thank you very much for the beginning of the compression!

Maybe you could implement a first implementation from lifthrasiir in #254 This should reduce the dictionary even more 🤔 Or we do it in another PR which is totally fine too!

Well, the sorting would have to be done in the generation scripts where word rankings are still available as the jsons only contain lists without rank score. I think that it's better to do that in a separate pull request.

@MrWook MrWook merged commit 3c304eb into zxcvbn-ts:master Sep 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants