Add more definition for Typescript

[maxime4000]

Update packages

44

[everweij]

Hi @maxime4000 , thanks for your PR, really appreciate it 👏🏻.

This is very helpful, this project could use some dependency updates, and I'm happy with the changes in typings (couple of names and exports). I am a bit worried about exposing internal implementation details, like useElementRef and useTrackElementResize to the outside world (see index.tsx). Can you elaborate on what's the rationale behind this choice?

[maxime4000]

Actually, I didn't give a lot of thought about why to not exporting them. I took the time to understand them before adding it to the export in the index.tsx and thought they were nice features that I could probably reuse later in the library I am working on. I didn't use them in the end, but they make me learn some few tricks I could do with React+Typescript. I wouldn't mind reverting that, but the thing about exposing useElementRef is that you can still reference it in a other way, by using import from "react-laag/dist/ToggleLayer/useElementRef" or something like it. But I didn't like to import features this way because I need to add multiple import to get all the type I need to make my components. Why would I made 5 import statements if I can make one that does all, so I try to export most of the type that was at least somewhat useful in the index.tsx. I also rename the Config to HoverOptions because it feel more explicit when you import them.

Also I would like to know if you were able of passing the test:ci script. I couldn't. Result always bigger than expected. I had to remove the test:ci script from the build script so I could yarn add from a git repo as temporary solution. I even try to update most of the dependencies to see if newer versions of chrome would fix it, but instead I had the same error with some new ones. Also I didn't understand why coding in Typescript if you transpile the code with babel instead of the Typescript compiler and generates the definition with Typescript compiler. I did come out with a solution where rollup was running Typescript compiler but I no expert on esmodule, commonJS, and I thought I broke something so I didn't push those changes.

Anyway, I just want to said, that I think this lib is very well made and it's rather easy to use. I'm loving it! I rewrote our popover/tooltip components using react-laag for our in-house components lib in one day and it's definitely an upgrade. I just think that it was shame that the properties were not enough exposed, so I fix that!

Other thing, I think your website working examples vs the codesandbox version of the examples is not the same, but I might be wrong. The example on tooltip doesn't use useTooltip which is a shame, but I was able to use it anyway because the good documentation. Really helpful. Good work on that :)

[everweij]

Actually, I didn't give a lot of thought about why to not exporting them. I took the time to understand them before adding it to the export in the index.tsx and thought they were nice features that I could probably reuse later in the library I am working on. I didn't use them in the end, but they make me learn some few tricks I could do with React+Typescript.

Yes, I understand why you did it. On the other hand, I'm concerned this might confuse other users. When they install react-laag and try to import things which can't be found in the documentation. I feel that by going this route I have to provide support for these hooks as well. Do you mind reverting these exports?

Why would I made 5 import statements if I can make one that does all, so I try to export most of the type that was at least somewhat useful in the index.tsx. I also rename the Config to HoverOptions because it feel more explicit when you import them.

Yes, this can be frustrating. You could create a central file which imports all the things like "react-laag/dist/ToggleLayer/useElementRef" etc. That way, you can import all kinds of stuff elsewhere in your code:

import { useElementRef } from './some-file';

Also I would like to know if you were able of passing the test:ci script. I couldn't.

I checked this the other day, and on my machine the test are running fine. I have to admit I'm not satisfied with the way testing is setup -> very cumbersome. Currently I'm working on v2 of react-laag where this is a lot easier. So I expect this to be fixed shortly.

Anyway, I just want to said, that I think this lib is very well made and it's rather easy to use. I'm loving it! I rewrote our popover/tooltip components using react-laag for our in-house components lib in one day and it's definitely an upgrade. I just think that it was shame that the properties were not enough exposed, so I fix that!

Thanks, great to hear! 👍

Other thing, I think your website working examples vs the codesandbox version of the examples is not the same, but I might be wrong. The example on tooltip doesn't use useTooltip which is a shame, but I was able to use it anyway because the good documentation. Really helpful. Good work on that :)

Thanks for the notice. I will put this on my todo-list.

[maxime4000]

Yeah I understand that might be confusing. I'll revert that and make a index files in ToggleLayer. This should work : import {} from "react-laag/dist/ToggleLayer"

[everweij]

Thanks @maxime4000 for your work! 👏🏻👍🏼 I will release a new version (v1.8.0)