HospitalRun / components

Reusable React components used by HospitalRun
https://components.hospitalrun.io
MIT License
124 stars 184 forks source link

v2.0.1 fails build of @hospitalrun/frontend #583

Closed jackcmeyer closed 4 years ago

jackcmeyer commented 4 years ago

When using @hospitalrun/components v2.0.1 in @hospitalrun/frontend, the build fails.

🐛 Bug Report

https://github.com/HospitalRun/hospitalrun-frontend/runs/1088553167

Expected behavior

The build should not fail.

Technical Notes

Caused by #581

DouglasDev commented 4 years ago

@jackcmeyer Perhaps the SCSS should all be moved to the scss folder? Then we wouldn't need to reference the src folder when importing.

CreativeCreate commented 4 years ago

Can I pick this with the following questions? Please bear with me if am asking the obvious here. I am just getting to know the @hospitalrun codebase.

Current npm package - @hospitalrun/components - ships raw *.scss files. 

  1. Assuming the components only get modified via props, is there a need for .scss files in the front-end other than the _variables.scss? I could not trace any ties in the front-end to these .scss files. ( front-end only uses a single .css file)

  2. Therefore, can we inject all component/*.scss into js as css at the component level (during the build?)

         // ex: in Toaster/index.tsx
          import 'react-toastify/dist/ReactToastify.min.css';
          import './toaster.scss';

    This way front-end does not need to do any .scss reference as these will be picked up by imports in the @hospitalrun/components/dist/components.esm.js.

  3. Also, @hospitalrun/components/scss/_variables.scss can be shipped for variable reference in the front-end if needed.

I tested this with both @components and @front-end successfully. And both worked w/o any errors and passed all tests. Would like to know everyone's thoughts on this as well. Thanks.

fox1t commented 4 years ago

@CreativeCreate Go for it! I think this is the best possible fix at the moment!

I really like what you've written. Your assumptions are correct!

CreativeCreate commented 4 years ago

Please note: now that there's no need for .scss references in the front-end, the main.scss reference in the front-end must be removed as follows.

   ...
   import { Provider } from 'react-redux'
 - import '@hospitalrun/components/scss/main.scss'    // ***to be removed
 - import './index.css'                               // ***include after App import
    import App from './App'
 // ***must import after App to override component styles
 + import './index.css'
  ...
 ReactDOM.render(

Not sure whether I should make a pull-request for the above front-end changes w/o getting assigned? Should I?