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

Release of v3.0.0 #184

Open
meyfa opened this issue Apr 30, 2022 · 21 comments
Open

Release of v3.0.0 #184

meyfa opened this issue Apr 30, 2022 · 21 comments

Comments

@meyfa
Copy link

meyfa commented Apr 30, 2022

Hello, I think it's quite awesome that this package is now written in TypeScript! Since it's been over half a year since the last v3 canary, I would like to ask whether a final release is planned yet.

@inca
Copy link

inca commented Jun 17, 2022

Any news on this? Please let me know if anyone needs any help moving this forward, I'm happy to help.

@leerob
Copy link
Member

leerob commented Jul 11, 2022

We're thinking about pushing the canary to stable - have y'all been able to use the canary okay without any issues?

@meyfa
Copy link
Author

meyfa commented Jul 12, 2022

Yes, the canary release works great. Only downside is that the plain string type isn't accepted, which makes ms a bit of a pain when used for parsing configuration files. Then again... passing invalid strings was already somewhat undefined behavior, and now it's made crystal clear to the user. If only there was a type predicate for narrowing string to StringValue, or a utility function that accepts string and returns a distinct (and documented) value in case parsing failed... But if that's not something you'd like to add I'd understand and I'll just continue working around it with type assertions.

@HendrikJan
Copy link

The whole usecase of ms for me is to convert configuration files, so if string is dropped, I'll have to look for another package.
The comment from @meyfa tells me that string is not accepted anymore in v3, is that true?

@dereekb
Copy link

dereekb commented Aug 11, 2022

String looks like it is still accepted. I'm using strings on the canary build right now.

They added StringValue to 3.0, and the documentation essentially recommends wrapping the ms() function with another function that has typed input for type safety.

https://github.com/vercel/ms/tree/3.0.0-canary.1#typescript-support

@meyfa
Copy link
Author

meyfa commented Aug 12, 2022

Maybe I've overlooked something obvious, but it seems to me that string is not accepted, i.e. the following fails typechecking:

const foo: string = '10s'
ms(foo)

"TS2769: No overload matches this call."

Wrapping the function as suggested by docs would be an okay solution, if in fact ms() threw consistently for every case of invalid input. It throws for empty strings, non-finite numbers, and anything that isn't a number or string, but for generic strings it returns NaN instead of throwing:

ms/src/index.ts

Lines 93 to 95 in 1304f15

if (!match) {
return NaN;
}

This means the type safety via try-catch is simply an illusion. Moreover, I couldn't find this behavior documented anywhere.

We'd have to use something like:

function ms2 (value: string): number | undefined {
  try {
    const parsed = ms(value as StringValue)
    if (Number.isNaN(parsed)) {
      return undefined
    }
    return parsed
  } catch (error) {
    return undefined
  }
}

which is quite a bit of boilerplate to include in every project, just for ms.

@aurelien-brabant
Copy link

I confirm that string is no longer accepted, which is unfortunate.

@dereekb
Copy link

dereekb commented Aug 12, 2022

Ah oops, my mistake. I went back and double checked the typings and reread the documentation.

You could just wrap the ms function with another and use it if you're going to use it in a lot of places:

import ms, { StringValue } from 'ms';

export function mss(value: string) {
  const x: string = 'example';
  return ms(x as StringValue);
}

They show this in the docs too. Behavior looks like it should still be same as the current version where an error gets thrown with bad input.

@meyfa
Copy link
Author

meyfa commented Aug 12, 2022

Behavior looks like it should still be same as the current version where an error gets thrown with bad input.

That's true, the behavior is unchanged since the previous version. No error gets thrown in case the input string fails to parse, though (see my previous comment). This means your proposed solution is unfortunately unsafe. See also #123.

@clarfonthey
Copy link

clarfonthey commented Oct 11, 2022

If there's gonna be a major version bump, can we change m to min? 👀

One genuine helpful case for this is that I sometimes can't tell if m was just ms being cut off, or if it truly means minutes. Using min can help stabilise this, and then it also opens up the possibility of using mo for months.

@rbnayax
Copy link

rbnayax commented Dec 4, 2022

very stable here...

@skoshx
Copy link

skoshx commented Dec 9, 2022

Friendly bump! Canary has been working perfectly ever since it was released... I recommend pushing to stable :)

@theoludwig
Copy link

canary becomes the new stable version at this point. 😂

Just ship it! 😄

@leerob
Copy link
Member

leerob commented May 6, 2023

CleanShot 2023-05-06 at 09 30 43@2x

I'd still love a bit more adoption before marking as stable.

@redwert
Copy link

redwert commented May 24, 2023

What is needed to finish for a stable version? I am ready to help

@skoshx
Copy link

skoshx commented May 24, 2023

@leerob With all due respect, I don't think people will adopt canary more organically, because most people just automatically install types from @types/ms... As such, I think to effectively find the issues with the new release (if there are any) the package should just be released as stable.

Even issue #189 demonstrates, how easily people miss the notice in the README, and as such don't install v3.

@mrmckeb
Copy link
Contributor

mrmckeb commented Jun 16, 2023

Only downside is that the plain string type isn't accepted, which makes ms a bit of a pain when used for parsing configuration files. Then again... passing invalid strings was already somewhat undefined behavior, and now it's made crystal clear to the user. If only there was a type predicate for narrowing string to StringValue, or a utility function that accepts string and returns a distinct (and documented) value in case parsing failed... But if that's not something you'd like to add I'd understand and I'll just continue working around it with type assertions.

@meyfa, we're open to PRs if you have an implementation in mind! We can always ship incremental improvements as minor updates too.

Here are some alternatives:

// 1. Cast to `StringValue`.
import ms, { type StringValue } from 'ms';
ms(someVar as StringValue);

// 2. Use `StringValue` in your code.
import { type StringValue as MsStringValue } from 'ms';
interface MyArgs {
  time: MsStringValue
}

// 3. Build a small wrapper.
import ms, { type StringValue } from 'ms';
export function stringMs (str: string): number {
  return ms(str as StringValue);
}

@jimmy-guzman
Copy link

In my opinion, a stable version of v3 should include #191. It would also be nice to get a published version of this change.

@SuperchupuDev
Copy link

We're thinking about pushing the canary to stable - have y'all been able to use the canary okay without any issues?

hey @leerob, it's been more than a year and a half since you said this, any updates on why it hasn't been published yet? it looks very stable

@EnriqCG
Copy link

EnriqCG commented Mar 18, 2024

I'd still love a bit more adoption before marking as stable.

image

Since this comment a year ago, adoption has increased by almost 3x to 200k downloads/week. There isn't a notable number of issues created so it does not look like the package is anywhere close to unstable.

@SuperchupuDev
Copy link

hey @styfle, you seem to be one of the only active members of the repo as of right now, do you have any updates on the status of v3? see the comments above

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

No branches or pull requests