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

Added blocklist constant #5

Merged
merged 1 commit into from
Aug 9, 2023
Merged

Added blocklist constant #5

merged 1 commit into from
Aug 9, 2023

Conversation

vinkla
Copy link
Collaborator

@vinkla vinkla commented Aug 6, 2023

No description provided.

@vinkla vinkla mentioned this pull request Aug 6, 2023
@schodemeiss
Copy link

Ignorant question and happy to be wrong, but wont a blocklist such as this potentially cause BC breaks by just adding in new words?

For example, lets say that "ABCD" is allowed in v1.0.0. In v1.0.1 "ABCD" is now blocked, any ID's that used to work, now wouldn't. Or worse, where 123 may have generated "ABCD", now generates something different.

If we treat the alphabet as a "seed" -- adding a blocklist essentially adds another part to that seed. But in this case, the blocklist is being driven by the library and is likely to change. Which is different to the default alphabet, because that's been stable since version 1 of hash-ids.

@4kimov
Copy link
Member

4kimov commented Aug 7, 2023

@schodemeiss It's a good question, and you're right: there is a good chance that modifying the library's default blocklist will make encode function produce different IDs whenever a blocked match is found (it should be expected). Although decoding old and new IDs will still work.

I don't really have a solution for that. The only thing I can think of is doing a major version bump whenever updating the blocklist (so end users will explicitly "agree" to the changes). Another thing we should probably do is add a "notes" section to individual READMEs where we talk about these "gotchas".

Do you have any other suggestions?

@4kimov 4kimov marked this pull request as ready for review August 9, 2023 20:26
@4kimov 4kimov merged commit 600bb56 into main Aug 9, 2023
6 checks passed
@vinkla vinkla deleted the blocklist-constant branch August 10, 2023 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants