[GitHub] [superset] michael-s-molina commented on pull request #21094: chore: Extract common select component code

[GitBox <gi...@apache.org>]

michael-s-molina commented on PR #21094:
URL: https://github.com/apache/superset/pull/21094#issuecomment-1225678511

   > Hi @cccs-RyanK. Thanks for this! Before I dive into the code I have a general comment. I am not very fond of the idea of having constants, types and common components all within a common file. If we are opting for separation then I believe it makes sense to separate things a bit more.
   > 
   > I'd prefer to see a structure similar to this one:
   > 
   > * types.ts: Containing all the shared types
   > * utils.ts: Containing shared functions and constants
   > * BaseSelect.tsx: containing the shared component
   > * This is optional, but we could also have a styles.ts for the common styled components
   > 
   > @michael-s-molina what's your thought about this?
   
   What I generally do is to keep things in context and depending on the size of things, break them into related submodules. One thing I don't like is mixing things that belong to different components. For example, I don't like to have a `types.ts` that contains all sync and async types because they are not shared. When things get bigger, and I have many types or styled-components, then I generally create your suggested structure but under each component folder like this:
   
   ```
   AsyncSelect
     types.ts
     styles.ts
     test.ts
     index.ts
   Select
     types.ts
     styles.ts
     test.ts
     index.ts
   utils.ts (shared)
   ```
   
   If I have a few types or styled-components, I generally define them in the component.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org