fullcalendar / temporal-polyfill

A lightweight polyfill for Temporal, successor to the JavaScript Date object
MIT License
361 stars 14 forks source link

More human readable error messages #30

Closed Ayc0 closed 8 months ago

Ayc0 commented 8 months ago

I don't really know if this is an issue that should be opened here, or on the RFC. In the rfc I just saw those generic TypeError:

image

So I think the error message is handled by the polyfill directly.

When creating an object from an invalid, for instance:

Temporal.Duration.from({ days: undefined });
// Throws:
// TypeError: No valid fields
//     at refineFields (index.cjs:1309:15)
//     at exports.refineDurationBag (index.cjs:2498:30)
//     at toDurationSlots (index.cjs:2857:89)
//     at Function.from (index.cjs:3316:37)
//     at Timeline.tsx:22:21

My use case: I used it with Temporal.Duration.from({ day: 5 }), and it didn't tell me that "day" wasn't valid and that I should use "days" instead. And also another time with Temporal.Duration.from(durationObject) but all fields were empty.

What if instead, if non-valid fields are passed, the polyfill could day something like "day" isn't a valid field. It needs to be either "years", "months", …. Or Duration is created from an empty object. At least 1 field as to be a number to be valid

This is low priority, but it can make debugging easier with the polyfill

arshaw commented 8 months ago

Thanks @Ayc0. I agree that the error message should be clearer. It'd be easy to print out a helpful list of the valid options in the case. I'll get this done for the next release.

arshaw commented 8 months ago

This has been fixed in v0.2.3

will say something like:

No valid fields: days,hours,microseconds,milliseconds,minutes,months,nanoseconds,seconds,weeks,years
Ayc0 commented 8 months ago

Amazing 🤩 Thanks!!