SimpleSigner / react-bootstrap-drawer

Drawer (Sidenav) Inspired by React Bootstrap Documentation
MIT License
15 stars 3 forks source link

Problems with typescript because of missing d.ts #1

Closed richtera closed 2 years ago

richtera commented 2 years ago

Hi I am getting problems using this module, because it's not correctly declared or marked as a module. image and there is no d.ts. I created my own which works fine for compilation, but the editor is still complaining.

declare module 'react-bootstrap-drawer';
SimpleSigner commented 2 years ago

@richtera - this library is not written in Typescript and as such does not declare a module or otherwise provide types for consumption. There exists some robust answers on StackOverflow in regards to your class of error, but unfortunately I am not in your project and so cannot provide a concrete answer as to what is or isn't the correct solution for you here.

richtera commented 2 years ago

The ts Problem is more of an FYI but types are also not available in definitely typed and there is a missing forwardRef call in the overlay

SimpleSigner commented 2 years ago

but types are also not available in definitely typed

Understood. As I've said, this is not a Typescript-based project and as such no types are provided. I'm neither familiar with DefinitelyTyped nor involved in their work.

and there is a missing forwardRef call in the overlay

I'm not sure I follow, or even have an overlay in this project for that matter? Could you please point me in the direction of the missing call? If so I can get a fix out this weekend.

richtera commented 2 years ago

A functional component with a ref needs to call forwardRef. I think the ts problem is not important but all libraries should have types if possible

SimpleSigner commented 2 years ago

A functional component with a ref needs to call forwardRef.

Could you please link me to the file / line number and or provide more details? I just reviewed all of the files and I have no ref usage in this library whatsoever.

I think the ts problem is not important but all libraries should have types if possible

Ideally, absolutely. Truth be told a Typescript rewrite is possible with a reasonably small amount of effort. However, I am currently on getting 1.0.0 out the door for our focus, the Simple Signer application itself.

richtera commented 2 years ago

I'll make a quick Fork and show you. Basically any component that needs a ref like a button would need it.

SimpleSigner commented 2 years ago

Are you asking we add in support for the use of ref, a la react-bootstrap (See: Example)? The way the statement was phrased, I read it as "You are using ref but forgot to implement the function."

richtera commented 2 years ago

Could it be possible that the version in npm does use refs? I didn't add it but master didn't have it when I checked right now :) it's no big deal initially I just wanted to let you know. Sorry we ended up down a rabbit hole .... I figured it out.... your example usage of the component using Collapse requires a ref. image

SimpleSigner commented 2 years ago

your example usage of the component using Collapse requires a ref.

Insofar as I'm aware there's nothing about react-bootstrap and the Collapse component that would require the use of ref. Looking at the documentation for the component, the only thing that it has documented as required is children. Is there some a warning or error you are getting here?

Sorry we ended up down a rabbit hole

No worries, half the point of having a project open-source is to have discussion and improvement for issues not conceived of initially. The original code for this project was pulled from the react-bootstrap documentation site's source-code. As such, it was not written by the original authors with the intention of being a library (otherwise this project wouldn't exist) and so it was not written with types (or ref) in mind but rather a simple set of components to provide a drawer. Since it satisfied our needs, out the door it went.

As I mentioned earlier, I'm a bit heads down on the larger project (in addition to a trip this week) so it might be a week or two before I'm able to commit to getting a 2.0.0 released with these features introduced.

Thanks for bringing all this to my attention!

SimpleSigner commented 2 years ago

See Also: https://github.com/SimpleSigner/react-bootstrap-drawer/issues/3