Fix Prod Build Bug w/ Bootstrap & Updated Dependencies #59
Conversation
|
Visit the preview URL for this PR (updated for commit 93c2ca2): https://tcl-77-smart-shopping-list--pr59-update-deps-d8hg77f6.web.app (expires Fri, 11 Oct 2024 07:34:02 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b77df24030dca7d8b6561cb24957d2273c5e9d72 |
|
We were working in office hours to see if we could fix the prod link not working |
…p better, resolve bootstrap and css preprocessor setting for vite compiling of project
…oking for root DOM element and not getting it causing react minification error
…e one the homepage is using has the scss styling and made sure the unauthenticated used it as well
bbland1
changed the title
bbland1
requested review from
RossaMania,
adidalal,
alex-andria,
eternalmaha,
tannaurus and
zahrafalak
|
Honestly shoutout to yall @RossaMania for sparking the idea that it was a config issue in vite, @adidalal for the dependency updates and the resolver with the change in imports because it was needed, @alex-andria, @zahrafalak and @eternalmaha for staying after office hours and late to just bounce ideas and problem solve. Oh and Alex for finding the breadcrumb to look at the manual chunking. And in general for bouncing ideas and hearing me out as I tried to figure out where the bootstrap issue started! 🎉✨ |
There was a problem hiding this comment.
Wow amazing work! Great investigative skills and putting that research into an actionable and working solution. From my end in local and build preview, bootstrap is working!!
I did notice that there are some design flaws in the item list that if your team decides to fix in a later PR may be a good start.
Great work!
| if (id.includes("react-bootstrap") || id.includes("@restart")) { | ||
| return "vendor__react-bootstrap"; | ||
| } | ||
|
|
There was a problem hiding this comment.
WOOOO!! Great job continuing from where we left off Brianna!! I'm amazed by your research skills - as always - and am so happy that you found this fix after reading the Vite and rollup documentation.
There was a problem hiding this comment.
You really sprung it into action with finding the manual chucking commented out worked again!
| api: "modern-compiler", | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
These are the same changes from Falak's old PR?
There was a problem hiding this comment.
Maybe, it was one of the things that cute talks about when using sass and I didn't see it in there when I was writing the resolver and the manual chunks so I did add it as well. I'd have to look at the old PR 🤔 this was branched from the messed up main so it may have been in a PR but didn't merge 🤷🏽♀️
@alex-andria If its the black box, the scrolling with that filter area staying on top was implemented before we had colors and I have dark mode on my browser so I just picked a color that would let me see the that area floated on top while scrolling 😅 it should be changed as we fix the design. |
Description
As we discovered on Monday the production version of the app was mad 😡. It was working locally but not after getting built somehow. While the build steps were "passing" in Github Actions something was not right.
In office hours we updated some of the dependencies, but the build still wasn't working. Then we looked at how
bootstrap/react-bootstrapworked withviteand howsassneeded to be imported. We wrote a resolver in thevite configfile to be able to properly resolve thesassimports as we found in the documentation. After we got to those steps we did some manual testing of how the application was handling the building of it in themanualChunksseeing that without that there wasn't an issue with the app but it was saying the minification was a bit large.After finding that the
manual chunkingseemed to be a sticking point, I went through theviteandrollupdocumentation to see how it was handled. With the error focusing on aforwardRefissue I did a bit of googling and it seemed it was something used byreact-bootstrapand themanual chunkingwas chunking parts of thenode_modulesto minify it but was separating parts that thereact-bootstrapneeded in a dependency of it called@restartI added aguard clauseto themanual chunkingthat would bundle thereact-bootstrapand@restartinto the same group. That for rid of theforwardRefand I got areact minification #299that when looking it up said thetarget wasn't a DOM elementit was and changing the script tag in the html todefergot rid of that.Lastly I noticed there was a duplication in the
AuthnticatedNavBarand the one the app was using wasn't getting the.scssapplied to that so I did correct that here as well.Acceptance Criteria
Type of Changes
Updates
Before
After
Testing Steps / QA Criteria