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

Deprecate uuidv4('binary') interface #437

Closed
ctavan opened this issue May 6, 2020 · 4 comments · Fixed by #454, statisticsnorway/ssb-component-library#256, Imageus-OSS/server#33 or concourse/concourse#6684

Comments

@ctavan
Copy link
Member

ctavan commented May 6, 2020

Is your feature request related to a problem? Please describe.

In earlier versions of this library it was possible to pass a single first parameter into the v1() and v4() methods which would result in the method returning a new Array instead of a string representation of UUID.

Apparently [email protected] already deprecated this API but it seemingly never got removed.

Describe the solution you'd like

We could either just remove the legacy interface or decorate it with deprecation warnings.

@broofa
Copy link
Member

broofa commented May 6, 2020

It's been ages since this API was documented. 'Tempted to say this wouldn't even require a major release but it'll probably break something somewhere.

Sorry, 'should've removed this ages ago but I'd honestly forgotten "binary" was even a thing.

@LinusU
Copy link
Member

LinusU commented May 6, 2020

Since we just released 8.0.0 it's probably safe to remove in a minor 😅

Technically it's not documented so it wouldn't be "illegal" to remove it, and practically I cannot really se anyone using both 8.x and this feature 🤔

@ctavan
Copy link
Member Author

ctavan commented May 6, 2020

Hmm, I'm confident that somebody out there will be using the library in weird ways…

On the other hand, a quick query of the BigQuery dataset I once collected in https://github.com/tc39/proposal-uuid/tree/master/analysis revealed, that only 302 out of 78868 lines containing 'uuid' also contained the term 'binary'. That is of course only a very rough proxy.

I'm really not sure, the safe bet would be to make this a breaking change.

@ctavan
Copy link
Member Author

ctavan commented May 12, 2020

Looking at the types of bug reports we get in here recently I don't even believe that pushing this into a major version would save us from bug reports, so I think technically the well documented deprecation in https://github.com/uuidjs/uuid/tree/v2.0.1#deprecated-apis should be enough to justify putting this into a minor version update. I'd prefer that over producing yet another major version bump as people are already confuse by the quick succession of major version increases.

ctavan added a commit that referenced this issue May 24, 2020
In version 1.x of this library it was possible to call `v4('binary')` in
order to receive a byte array instead of a string representation. This
function signature was deprecated in 2.x (but not removed in 3.x as it
should have been).

The correct way to get a binary representation of a uuid is to pass an
array-like object as a second parameter:

```
const buffer = new Array();
v4(null, buffer);
```

Fixes #437.
ctavan added a commit that referenced this issue May 25, 2020
In version 1.x of this library it was possible to call `v4('binary')` in
order to receive a byte array instead of a string representation. This
function signature was deprecated in 2.x (but not removed in 3.x as it
should have been).

The correct way to get a binary representation of a uuid is to pass an
array-like object as a second parameter:

```
const buffer = new Array();
v4(null, buffer);
```

Fixes #437.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment