Missing exported components, hooks, utils, and types

[aleksanderd]

Hi!

While playing with the lib, I noticed some absent exports to re-use.
Why not to export as much as possible to be able extends on any level?
Also, what about to use named exports (instead of defaults) to simplify index-exports?
So, instead of

export { default as useLayoutNavigation } from "./useLayoutNavigation";

just export all:

export * from "./useLayoutNavigation";

I made some refactor for example on layout component, check it please, and if there is no objections from you, may be I can help with refactor.

https://github.com/aleksanderd/react-md/commit/c326fb2066a75d438f6466e55010520c2f5a29bb

Thank you!

[mlaursen]

Hey, great questions!

This is really just because of my personal preferences for how I keep track of files and exports and trying to keep some consistency. My general rule of thumb is:

• default export is always a React component, the main React hook, or a renderer function so that it
• export * will always be type information, reusable constants, or utils
• only do export { default as Thing } in the main index file

I also think it looks cleaner to just do import fileName from './fileName' than import { fileName } from './fileName' which maps nicely to my custom snippets.

The export { default as Thing } also helps a bit with how I've done the forwarded refs and keeping it consistent with non-forwarded components (here's a commit explaining this a bit - 61f33e8). However.. as I'm typing this out, I realize I could actually do:

export const MyComponent = forwardRef<HTMLElement, Props>(function MyComponent(props: Props, ref?: Ref<HTMLElement>): ReactElement {
  // ...implementation...
});

which would still allow me to add prop-types.

Finally, the main reason I haven't exported everything by default for complete re-use is that there are some components, hooks, utils, and types that I consider "internal" for react-md since I never designed them in a way to be used without the specific use-case they are tied to and I want to be able to change/break these without a major version bump -- I normally annotate those with @private. I normally document "public" apis and components heavily while the internal lack prop-types and documentation most of the time. The plan is to eventually expose more of these private apis and components, but it requires all the additional work of documentation and creating examples on the documentation site.

[aleksanderd]

Thank you for clean answer!

In my case I tried to use LayoutAppBar which is not exported, but I understood I need custom AppBar component anyway, so just copy-pasted LayoutAppBar for start)

[aleksanderd]

+ I missing/copypasted isToggleableLayout - is it private too, or may be just forgot to export? ;)

[mlaursen]

Hmmm. This is good feedback! It's mostly been me working on v2 so I didn't have any other use-cases than the default and didn't know if anyone was using it yet. I think I would be good with making more of these components and types "public", but maybe add a warning that they are likely to change without warning since they aren't fully documented.

I do think the documentation part will be easier once I figure out how to use typedoc, so maybe I'm just too strict about it right now. If you have the time to open a PR exporting a few more components, utils, and types with my existing conventions, I would happily merge it and do another alpha release for you! I'm still busy working on the changelogs and trying to focus on that -- I keep getting distracted...

Also: what's the use-case for the custom LayoutAppBar? This might be something I might've missed with the re-write and consider required for the full v2 release

[aleksanderd]

I just playing + learning with it and trying to create appbar with menu + some customs...

re-write and consider required for the full v2 release

some options for sub-components of Layout are missing, since there is some mapping for it(like appBarFixed => fixed prop of AppBar)...

why not to describe just few props for each sub-component, like:
appBar: ReactEl (rendered)
appBarConfig: AppBarProps (or extend/template)
appBarComponent: defaults to LayoutAppBar

make all sub-components props flat is not too good idea imho... we already have types for AppBar props, why not use it where we need? simplier writing/typing/overriding/understanding... no outdated props maps ;)

[mlaursen]

Those changes sound good to me since it could also be applied to the <main> component and since there are too many props being passed down now. I think I'll eventually apply this to other packages/components as well that have sub-components.

[mlaursen]

I looked over this package again while writing the changelogs and realized it was never finished and was just hacked together to get the documentation site able to render. The latest commit (0dec760) should address some of the missing customization and I tried out the new <name>Props pattern which seems to work out pretty well. All the constants, types, utils, and additional components should now be correctly exported as well as some other few changes.

I'll be updating the documentation a bit within the next few days, but the nice thing about these changes is that the Layout no longer requires the navItems and can be rendered as just a simple layout instead of a navigable layout. It also fixed some of the keyboard focus-loss and keyboard trap behavior that shouldn't have been there.