search

gpbl / react-day-picker

DayPicker is a customizable date picker component for React. Add date pickers, calendars, and date inputs to your web applications.
https://daypicker.dev
MIT License
6.09k stars 731 forks source link

React DayPicker 8 #942

Closed gpbl closed 3 years ago

gpbl commented 5 years ago

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

Notes

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 ❤️

shelver2004 commented 5 years ago

hello @gpbl i need some help with captionElement for 2 calendars for date range, how can i have changeable month and year dropboxes? do you have an example for date range date picker?

KyorCode commented 5 years ago

Will there be a useInput hook for ranges?

gpbl commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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.

I think remove the peerDependency is the way to go.

milesj commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

gpbl commented 5 years ago

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?

milesj commented 5 years ago

That proposal is fine, assuming it's not using date-fns. The problem arises when date-fns and luxon (or moment) are used in parallel, as the bundle size will include both, when we ideally only want 1.

I think there was just a misunderstanding of the separate package nomenclature. Basically there are 3 proposals being discussed here.

1) Keep it like v7 where all date logic is handled manually.

2) Offload all date logic into props/utils and require consumers to configure it.

3) Like 2 but also provide the utils as separate packages: react-day-picker-date-fns, react-day-picker-luxon, react-day-picker-moment, etc.

My vote is for 3. I can help provide the luxon utils if need be.

stunaz commented 5 years ago

vote 3 - react-day-picker-dayjs

tnyuan commented 5 years ago

We are also using react-day-picker with luxon, and would prefer not to have date-fns as dependency by default. Option 2 is good enough, option 3 can come later if needed.

stunaz commented 5 years ago

IMHO option 2 will generate a lot of issue.

gpbl commented 5 years ago

Thanks everyone for the feedback. I will find a way out...

KyorCode commented 5 years ago

The react-day-picker-utils could actually be a sort of plug & play package, you just pass the exported object from that package as a prop to your RDP or you implement it yourself. Doesn't have to be a dependency.

Within RDP you'll only have to check whether the required date/time function is present. In other words kind of enforce people too provide the utilities either by using your package or by implementing it themselves. Documentation on what each util function is supposed to return is key here. I guess you could fallback on the documentation in v7 for that.

porfirioribeiro commented 5 years ago

I think using date-fns vs using own functions is better Because if you use own functions they will still end in bundle and if we use date-fns on our code we will have duplicated code. Also date-fns is three shakeable so only the used functions will end up in bundle. Using date-fns or own functions will probably give the same bundle size

The other option is keep all functions as props, even tho you will end up with lots of prop drilling probably

gpbl commented 5 years ago

Using date-fns or own functions will probably give the same bundle size

This is my point as well.

However the functions from date-fns seem to increase the bundle to 15KB and it is a bit too much. I need to investigate, maybe is the rollup config, or the locale bundle.

I understand the selling point of RDP being library agnostic, but:

It should be possible however with webpack resolve date-fns to a bridge dependency, replacing the date-fns functions with other libraries' wrappers.

shelver2004 commented 5 years ago

@gpbl My apologies if this question doesn't belong to this subject. Is there Any example to highlight the 'from' nd 'to' days when user pass those values in props?

porfirioribeiro commented 5 years ago

@gpbl Try to change imports to direct imports, i also had problems with esm imports

import { isToday } from 'date-fns';

to:

import isToday from 'date-fns/isToday';

That way it will only import that function

gpbl commented 5 years ago

@porfirioribeiro thanks! I think importing that way will help also to use a webpack resolver.

I've made some analysis, right now it actually works great. We'd just add 2KB compared to the previous version:

Computed sizes of "lib/index.umd.min.js" with "umd" format
  bundler parsing size: 60,246 B
  browser parsing size (minified with terser): 60,111 B
  download size (minified and gzipped): 13,884 B

Also useful is seeing the download count date-fns vs luxon vs moment vs days:

https://npm-stat.com/charts.html?package=luxon&package=date-fns&package=moment&package=dayjs&from=2018-10-30&to=2019-10-30

Screenshot 2019-10-31 at 06 05 18

As conclusion, I will keep v8 with date-fns as dependency. If those 2KB difference from previous version are really important, someone will help with a PR.

porfirioribeiro commented 5 years ago

Great! I'm glad to help!

I'm waiting for this release to implement in my application in development now. It still only has a date input manager

If there is anything else i can help

henryruhs commented 5 years ago

Is there a roadmap for Alpha and Beta releases on NPM?

Allow me to suggest a better project structure - at this point the files are widely distributed and a component based approach is hard to figure out.

henryruhs commented 5 years ago

Just take a look into an Angular project and you figure out some best practices. TypeScript and React go well together - this project looks very promising to me. Keep on improving 👌🏻

I refactored Day.tsx to proper usage of TypeScript as you are bypassing most of it's power:

import * as React from 'react';
import { getDayProps } from './helpers/getDayProps';
import {
    PropsInterface,
    dayPropsInterface,
    containerPropsInterface,
    wrapperPropsInterface
} from './DayInterface';

// introduce proper interfaces in ComponentInterface.tsx for each Component.tsx
// put all const to the top - leave blank line after const/let blocks
// avoid using { xxx } = zzz as this bypasses TypeScript validation
export const Day: React.FC<ReactDayPicker.DayProps> = (props: PropsInterface) => {
    const { day, modifiers, dayPickerProps } = props;
    const { locale, formatDay } = dayPickerProps;
    const dayProps: dayPropsInterface = getDayProps(
        day,
        modifiers,
        dayPickerProps
    );
    const containerProps: containerPropsInterface = dayProps.containerProps;
    const wrapperProps: wrapperPropsInterface = dayProps.wrapperProps;
    const Component = modifiers.interactive ? 'button' : 'span';

    if (modifiers && modifiers.hidden) {
        return <span />;
    }
    return (
        <Component {...containerProps}>
            <span {...wrapperProps}>{formatDay(day, { locale })}</span>
        </Component>
    );
};
ZWkang commented 5 years ago

may i help you write some test cases ? Version 8 is a breaking change?

KyorCode commented 5 years ago

Is there a roadmap for Alpha and Beta releases on NPM?

Would be very interested to know this :)

kuehlein commented 4 years ago

Is there a certain commit / branch that we could use to link to as a dependency? I have a demo coming up pretty soon and I'd like to be able to utilize some of the fixes in v8 prior to the demo.

KyorCode commented 4 years ago

@kuehlein Master is V8 at the moment. If you install it throug a git reference and update the package.json a bit. You can get it to work.

We currently use the following in our package.json: "react-day-picker": "gpbl/react-day-picker#master"

and run the following script to fix the package.json reference.

const path = require("path");
const fs = require("fs");

const reactDayPicker_Path = "./node_modules/react-day-picker";
const reactDayPicker_Package_path = "/package";

const valuesToUpdate = ["main", "module", "typings"];
const arrayValuesToupdate = ["files"];

try {
  if (fs.existsSync(reactDayPicker_Path)) {
    const pkg = JSON.parse(
      fs.readFileSync(path.join(reactDayPicker_Path, reactDayPicker_Package_path, "package.json"))
    );
    valuesToUpdate.forEach(key => {
      if (!pkg[key].includes("package")) {
        pkg[key] = `package/${pkg[key]}`;
      }
    });
    arrayValuesToupdate.forEach(key => {
      pkg[key] = pkg[key].map(val => {
        return val.includes("package") ? val : `/package${val}`;
      });
    });
    fs.writeFileSync(path.join(reactDayPicker_Path, "package.json"), JSON.stringify(pkg, null, 2), {
      encoding: "utf8"
    });
  }
} catch (err) {
  console.error("react-day-picker has not been installed.");
}
evadecker commented 4 years ago

Just wanted to pipe in and say I fully support the decision to use date-fns as a dependency given its small size and the issues you flagged above! Thank you so much @gpbl for all of the time and effort you have put into (and continue to put into) this library! I'm using it for my 2nd project and it is a joy to work with, unlike basically every other date picker out there. Thank you.

evadecker commented 4 years ago

In addition, for those of us without the time or expertise to contribute to the code but who would like to thank you with a few bucks for a coffee or a beer, do you have a Venmo or Paypal we can send dollar bills to? 💵

denkristoffer commented 4 years ago

date-fns has had a few critical but very hard to debug issues with e.g. timezones (such as this one) and while it isn't really fair, it has made me lose some trust in it. I've moved my own projects to dayjs instead and would be cautious before requiring date-fns.

Eric24 commented 4 years ago

Love the library! Why don't you add a Github Sponsor option? I'd be happy to contribute, and I suspect many others would, too.

gpbl commented 4 years ago

Thanks everybody for the nice responses! Some updates:

I'll consider a sponsor, thanks for the idea!

denisborovikov commented 4 years ago

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

gpbl commented 4 years ago

@denisborovikov looks promising! thanks for finding it out !

bengry commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@Bangry thanks for your time ! Appreciate the comment. I don’t want to spam the subscribers here - I've opened an issue https://github.com/gpbl/react-day-picker/issues/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 commented 4 years ago

@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 commented 4 years ago

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

ivansky commented 4 years ago

@gpbl Why tslint instead of eslint? As you may know TSLint is deprecated. https://github.com/palantir/tslint/issues/4534