Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

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

Closed
jackcmeyer opened this issue Sep 8, 2020 · 4 comments · Fixed by #593
Closed

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

jackcmeyer opened this issue Sep 8, 2020 · 4 comments · Fixed by #593
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@jackcmeyer
Copy link
Member

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

@jackcmeyer jackcmeyer added bug Something isn't working help wanted Extra attention is needed labels Sep 8, 2020
@DouglasDev
Copy link
Contributor

@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
Copy link
Contributor

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.

  1. 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
Copy link
Member

fox1t commented Sep 14, 2020

@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
Copy link
Contributor

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? 

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
4 participants