Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

React DayPicker 8 #942

Closed
gpbl opened this issue Oct 8, 2019 · 53 comments
Closed

React DayPicker 8 #942

gpbl opened this issue Oct 8, 2019 · 53 comments
Labels
help wanted Extra attention is needed in progress Work in Progress
Milestone

Comments

@gpbl
Copy link
Owner

gpbl commented Oct 8, 2019

v8 will be a TypeScript rewrite and will introduce some major, needed changes.

Testing v8

Add the next version to the dependencies:

yarn add react-day-picker@next

v8 Objectives

  • Rewrite the package to follow the latest React patterns
  • Improve API and customization with custom components
  • Resolve TypeScript typing issues by rewriting it in TypeScript
  • Replace DayPickerInput with a more flexible alternative, e.g. an hook.
  • use date-fns as date utility peer dependency
  • have a browserstack functional tests
  • have a new website

Notes

  • DayPickerInput seems very popular and removing it can make difficult the upgrade.
  • Need to be React +16.8
  • Likely we will need to maintain v7 for a while

Please help 🙏🏾

With more than 1M download per month, react-day-picker needs love

It is time consuming to work alone on this project, organize the repository, write documentation, the functional tests, and the upgrade guides (not native speaker here).

Please help, talk on https://spectrum.chat/react-day-picker

I started this project five years ago as a way to contribute to the awesome React community. I've learnt a lot, I had a lot of fun, and met the best developers thanks to it.

However, after all these years, the old codebase is not much fun to maintain and after so many changes, the code is not as good as it should. Developers started filling issues on Github, and the glorious OSS experience I had so far hit me to the point I didn't want to read Github notifications anymore.

Surprisingly tho, downloads never stopped to grow and this library is still popular! People continue to send PRs, and some even wrote me that it would be sad to see this library to die. These nice messages have been highly motivational so I'm here again ❤️

@gpbl gpbl pinned this issue Oct 8, 2019
@gpbl gpbl added the v8 label Oct 8, 2019
@gpbl gpbl added this to the v8.0.0 milestone Oct 8, 2019
@gpbl gpbl removed the v8 label Oct 8, 2019
@gpbl gpbl changed the title v8 release status 🚢 Upcoming Release 🚢 Oct 10, 2019
@gpbl gpbl changed the title Upcoming Release 🚢 Upcoming Release v8 🚢 Oct 10, 2019
@shelver2004

This comment has been minimized.

@gpbl gpbl changed the title Upcoming Release v8 🚢 Upcoming Release v8 🚢📆 Oct 23, 2019
@KyorCode
Copy link

KyorCode commented Oct 25, 2019

Will there be a useInput hook for ranges?

@gpbl
Copy link
Owner Author

gpbl commented Oct 28, 2019

@KyorCode please give a look:

https://rdp-v8.netlify.com/docs/input

For sure, useInput can be extended to support ranges. What is your use case?

@milesj
Copy link

milesj commented Oct 28, 2019

The reason I chose this library is because it doesn't force the consumer to use a specific date/time library (we use Luxon). I'm not a fan of use date-fns as peer dependency, unless it's an optional peer dep.

@KyorCode
Copy link

@gpbl My use case went out of the window after some talking to the design team. They wanted to have a single input field for a date-range-picker which in practice disables the user from input unless we use a mask or some sort. So we split up the range into two separate inputfields and problem solved :)

@milesj We too did pick this library over others because it had no dependency on any date/time library. The localUtils worked great for us, really sad to see them go actually.

@shelver2004
Copy link

@kryops @gpbl hi guys I'm just about to use react-daypicker for our application..just saw @kryops comment about being sad to see them go..what do you mean? Is this library still available?

@KyorCode
Copy link

@kryops @gpbl hi guys I'm just about to use react-daypicker for our application..just saw @kryops comment about being sad to see them go..what do you mean? Is this library still available?

The library is going through an overhaul and one of the changes is that it is going to have a dependency on date-fns as a date/time utility library. The v7 version (and previous versions) had the possibility to use your own choice of date/time utility library ( globalize, Luxon, moment-js, ... ). Seeing that option go is making me sad, I'm actually happy to see all the other changes though and really appreciate all the work @gpbl is putting into this!

@gpbl
Copy link
Owner Author

gpbl commented Oct 29, 2019

Wow thanks for the feedback! Finally someone rising a voice :)

At the time I wrote the component, there was no date-fns, and moment was the only option. Since I did not want to lock people using it, I decided to write my own date utilities – and i tell you it has been a nightmare and a waste of time.

Now we can include date-fns in the package without the issues we had with moment, and rely on the amazing solid work of other people.

  • If your concern is adding a new dependency to your stack, we can add date-fns as internal dependency.
  • If you really want to use your same date utilities, then I need help for making RDP library agnostic. But I don't see a reason for doing so, and I wouldn't for the first release.

I think remove the peerDependency is the way to go.

@milesj
Copy link

milesj commented Oct 29, 2019

@gpbl A few things.

  1. What were the pain points in managing the dates yourself? I'd possibly be open to hardening the API. Another option would be to completely avoid any date logic, and require consumers to implement the API on their end using props, while the picker uses the result (a Date object) for display only.

  2. When we were researching available datetime libraries, we went with Luxon over date-fns, as the latter does not support timezones out of the box. Ideally everything in the future uses temporal.

@gpbl
Copy link
Owner Author

gpbl commented Oct 29, 2019

What were the pain points in managing the dates yourself?

Many :) Locales, timezones, etc. I don't want to deal with this things anymore.

Maybe exposing date utilities API as wrapper to native date-fns would work. I'd appreciate some help implementing it:

<DayPicker
  dateFns={{
   isToday: day => day.getDate() === new Date().getDate()
  }}
/>

As alternative a babel-plugin or a webpack resolver would work too!

@milesj
Copy link

milesj commented Oct 29, 2019

Yeah possibly. Do you have a list of all the datetime methods that would be necessary?

Another option would be to turn this into a monorepo, where additional packages are all the datetime adapters.

@gpbl
Copy link
Owner Author

gpbl commented Oct 29, 2019

Not many methods, but few of them are tricky (like the number of the week according to locales).

https://github.com/gpbl/react-day-picker/search?l=TypeScript&q=date-fns

but really what is the difference including own-written utilities, or those from another package? What are the main concerns? Maybe the locale object being too big?

@stunaz
Copy link

stunaz commented Oct 29, 2019

but really what is the difference including own-written utilities, or those from another package? What are the main concerns?

People don't want 2 date/lib in their bunlde.

@gpbl
Copy link
Owner Author

gpbl commented Oct 29, 2019

People don't want 2 date/lib in their bunlde.

So, if not a peer dependency, then an internal dependency would make everybody happy?

@milesj
Copy link

milesj commented Oct 29, 2019

No, because it would still end up in the bundle.

@gpbl
Copy link
Owner Author

gpbl commented Oct 29, 2019

If not a peer dependency, then an internal dependency would make everybody happy?

No, because it would still end up in the bundle.

So, let say we continue with our own, internal utilities – like in the v7. No external date lib.

Now, such utilities are becoming good enough to stay in their own package, let say react-day-picker-utils. Thus they will become an internal dependencies – pretty much like i propose for date-fns.

Why this solution would be different? What is the actual problem of including a dependency? Size is not, stability either. So what?

@denisborovikov
Copy link

@gpbl Have you seen date-io, abstraction over common javascript date management libraries?

@gpbl
Copy link
Owner Author

gpbl commented Jan 23, 2020

@denisborovikov looks promising! thanks for finding it out !

@bengry
Copy link

bengry commented Jan 23, 2020

@denisborovikov @gpbl seems like the core library itself + the adapters make it not very slim. It would probably be better to depend on something like date-fns, which is modular by nature, and this project would only be using a handful of functions from there, which will probably be smaller then the date-io core + adapter package. I'm also leaving aside the addition library that the app may use (moment/date-fns/luxon etc.) for other parts of the app.

Not saying this is a bad library, I just don't see the major benefit it gives over using date-fns, which is just an implementation detail in this case, since you still expose native Date in the components' APIs.

@denisborovikov
Copy link

denisborovikov commented Jan 24, 2020

@bengry I think it makes sense to check the real numbers before making a decision. date-io/core is only 100 LOC, any adapter is around 300 LOC. date-fns is 79.3kB minimized, but of course, it depends on how many functions react-day-picker uses.

@bengry
Copy link

bengry commented Jan 24, 2020

@denisborovikov bundlephobia wasn't able to analyze @date-io/core. Looking at the code it seems like it doesn't have any runtime by itself and only includes interfaces that the actual implementations (e.g. @date-io/date-fns) use. However, when including the adapter for the chosen library, you get an overhead of at least 1.2kB (gzipped and minified) (when using moment, others are larger by a bit).
Then, if you use a tree-shakeable package (like date-fns, and unlike moment. I'm not sure where Luxon and other libraries that date-io supports stand), you lose the tree-shakeable aspect of them, to some extent, since you wrap them all in a class (DateFnsUtils), which are not tree-shakeable AFAIK, so even if react-day-picker only uses a couple of functions there, the app using it will have to carry the load.

@denisborovikov
Copy link

@bengry Not sure I understand your point. react-day-picker doesn't have to include any date libraries. Here's an example of the library, that already uses it. You have to pass an adapter instance through the context.

If your project already uses moment.js, it's your project's problem if it's tree-shakeable or not, and you probably already resolved this issue or can deal with it.

@bengry
Copy link

bengry commented Jan 24, 2020

@denisborovikov if a project uses moment.js then they add 4.1kB to their minified bundle (before gzip). That's the easier case though. If a project uses date-fns, they add 5.9kB to their minified bundle, plus functions that neither them nor react-day-picker may need. For example, since date-io has a getDiff method, then the date-fns adapter will also implement it. For the sake of the argument, let's say that neither my app nor react-day-picker need this function - if we were both to use just date-fns, it would get tree-shaked away. However, because of the user of date-io, it can't, since it's part of a class that react-day-picker uses, so it's bundled as a whole.

The getDiff function is just an example, there are a dozens of other functions that date-io requires as part of their interface, and are therefore bundled in, and I'm sure most of them are unused by react-day-picker. We can dig into the source code of v8 (I haven't had the chance yet) to see exactly what it needs - but I assume it needs just a handful of them.

P.S.
since date-io has a unified interface, it covers only some of the stuff that react-day-picker potentially needs, so even this abstraction will break at some point, especially if @gpbl wants to add timezone support, in any extent, something that's not covered by date-io whatsoever as far as I saw.

To sum things up, I prefer to keep this (and any other) libraries clean from dependencies. The abstraction that date-io adds is nice in theory, and if this were a backend project maybe I wouldn't care that much about the bundle size, but keeping things lean in frontend packages is very important, in my opinion at least. That is not to say that you shouldn't have dependencies in it, but date-fns makes for a good candidate, because it is tree-shakeable, meaning that only what's used gets bundled, and thus it would've been the same size anyway as a whole if these functions were part of react-day-picker itself (as they are today). Before using this, we used https://github.com/airbnb/react-dates, which has moment as a peer dependency, so just to use the date picker we had to add 220kB + 231kB to get a date picker. This is one of the reasons we moved to react-day-picker - exactly because it's very lean, meaning we can ditch moment if we don't need it anymore.

I hope I managed to get my point across. LMK if there are still misunderstandings or I wasn't clear about something.

@gpbl
Copy link
Owner Author

gpbl commented Jan 26, 2020

@bangry thanks for your time ! Appreciate the comment. I don’t want to spam the subscribers here - I've opened an issue #980. There are many ways to keep the package size small - is my design constraint - but I won’t write any date utility anymore- it’s ton of demotivating work :)

There’re other interesting design decisions to take. Eg i want to remove the idea of “modifiers” - this is a old heritage of the CSS BOM syntax I was using years ago. WTF is a modifier? How do i name this type? - and so on. Why not a more declarative way?

I need also help with docs, code reviews, issues triaging... so i can keep working on the code :)

Is donating a way to motivate people enough? Not sure i like these things in OSS, where to me motivation is the challenge of writing good code / DX, learning new stuff - without the constraints of work hours or schedules.

@bengry
Copy link

bengry commented Jan 26, 2020

@gpbl Please do edit your comment with a link to the issue once opened, I'll be happy to continue the discussion there. I also agree maintaining the date utilities part of this library is tedious, and also very error-prone. Dates are nothing to take for granted, especially on the web, and libraries like moment or date-fns are popular for a good reason.

Regarding the other things you raised, like modifiers etc. - these are all things worthy of their own discussion IMO. I like the idea of modifiers, I just think that tying it to the CSS class names is the issue. We use CSS Modules in our project, and although things work fine, there are broken things because of it (e.g. #959).

I suggest maybe opening issues for each respective aspect of v8, and maybe grouping them via a milestone so it's easier to discuss things separately. I'd be happy to take part in some of these, and maybe contribute some code as well, if it's something you're open to.

@gpbl gpbl changed the title Upcoming Release v8 🚢📆 (v8): Next mayor release Jan 27, 2020
@gpbl
Copy link
Owner Author

gpbl commented Jan 27, 2020

Thanks @bengry, I'm setting up a project here: https://github.com/gpbl/react-day-picker/projects/7, issue about date-library is here: #980

@ivansky
Copy link

ivansky commented Feb 21, 2020

@gpbl Why tslint instead of eslint? As you may know TSLint is deprecated.
palantir/tslint#4534

@samueldosramos
Copy link

Do you intend to put hour and minute adjustments in v8?

@gpbl
Copy link
Owner Author

gpbl commented Apr 19, 2020

Do you intend to put hour and minute adjustments in v8?

@samuelramox this is a feature request I got many times, definitely we should evaluate to add it – not in the first release tho.

@gpbl
Copy link
Owner Author

gpbl commented Apr 22, 2020

Good news everybody!

I could publish a first alpha version on npm:

yarn add react-day-picker@next

I'd love someone helping me, as I will be busy in the next weeks.

  • documentation, proof reading
  • PR reviews
  • pick up something from the project

@gpbl gpbl removed this from the v8.0.0-beta milestone Apr 27, 2020
@gpbl gpbl added help wanted Extra attention is needed in progress Work in Progress labels Apr 27, 2020
@gpbl gpbl changed the title (v8): Next mayor release Next mayor release: v8 Apr 27, 2020
@gpbl gpbl changed the title Next mayor release: v8 📅 Next mayor release: React DayPicker 8 Apr 27, 2020
@gpbl gpbl changed the title 📅 Next mayor release: React DayPicker 8 React DayPicker 8 Apr 27, 2020
@gpbl gpbl added this to the v8.0.0 milestone Feb 10, 2021
@gpbl gpbl closed this as completed Feb 10, 2021
Repository owner locked and limited conversation to collaborators Feb 10, 2021
@gpbl gpbl unpinned this issue Feb 10, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
help wanted Extra attention is needed in progress Work in Progress
Projects
Status: Done
Development

No branches or pull requests