odysee-frontend
odysee-frontend copied to clipboard
Webpack upgrade
Fixes
Opening as a draft as I still need to do some deeper testing to ensure there are no quirks. But the basic funcitonality works as expected.
I'd expect any issues to surface at build time as opposed to runtime. But just being a user, I'm not familiar with all the nooks and crannies, so any extra eyes for any potential oddities would be good.
Upgrade webpack and related changes as an incremental step toward implementing typescript, as mentioned here https://github.com/OdyseeTeam/odysee-frontend/issues/1178.
Rolls in https://github.com/OdyseeTeam/odysee-frontend/pull/1901
Other information
Webpack and related updates were made. Not all of the packages could be brought to current. Specifically, postcss
and cssnano
. postcss-rtl
is no longer maintained and will not function with the latest major version of postcss.
PR Checklist
Toggle...
What kind of change does this PR introduce?
- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting)
- [x] Refactoring (no functional changes)
- [ ] Documentation changes
- [x] Other - Please describe: build upgrade
Please check all that apply to this PR using "x":
- [x] I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
- [TODO] I have checked that this PR does not introduce a breaking change
- [ ] This PR introduces breaking changes and I have provided a detailed explanation below
I've been poking at this over the past couple days, and everything I've seen is working. Let me know if any of you run into any issues.
Might want to hold off changes to see what Tom thinks -- is the timing right?
Of course. No rush from my end. Take it at your leisure or even use this as a baseline reference down the line if this is something that needs to be done much later.
Of course. No rush from my end. Take it at your leisure or even use this as a baseline reference down the line if this is something that needs to be done much later.
Cool. I think this is good for the path to typescript, though. Let's wait for his feedback on timing and testing bandwidth.
The changes regarding the images have been implemented. There is still one hanging issue with NODE_ENV being lost that I still need to resolve. My first guess is that it has something to do with some interaction between the webpack v4 and cross-env, but I'll get that worked out in my next pass (side note/FYI: cross-env is now in maintenance mode).
Inadvertently closed the PR. Reopened.
I wouldn't mind getting this in, but could use some testing on one of our test instances. Let's get it building first?
failing on Travis: https://github.com/OdyseeTeam/odysee-frontend/runs/7623226763?check_suite_focus=true
thanks for the effort @david0178418 !
@tzarebczan Anytime!
Yeah, in the next batch of free time I have, I'll figure out why NODE_ENV isn't propagating in the CI environment. I'd previously set it in the script, but had forgotten that this was the same script ultimately used for dev as well. I'm guessing it has something to do with some interaction between webpack and cross-env.
Also, it might require some trial and error commits since I can't seem to reproduce the issue locally. My apologies to you and the team if it spams you all with borked build messages while I feel that out.
@tzarebczan if you give me a heads-up before you plan on putting this on a testing environment, I can do another package update and rebase against master to make it as updated as possible before testing.
I can put it up today, so let me know when it's ready!
@tzarebczan Ok, you should be good to go now.
Got this during build: warning " > [email protected]" has incorrect peer dependency "webpack@^1 || ^2 || ^3 || ^4". [webpack-cli] Error: Unknown option '--display' [webpack-cli] Run 'webpack --help' to see available commands and options error Command failed with exit code 2.
That's coming from our build script - maybe just remove? not supported anymore?
Thanks. I'll see about taking a look at it later this evening.
I shipped it on stashu.odysee.com without that. Seems to work properly, no other warnings. We'll give it a whirl over next few days to make sure nothing major creeps up.
Sounds like a plan. Thanks guys.
So far things are looking good on the test site. Any things you can think of that would be problematic or to look out for, or basically since it builds, runs, should be fairly safe?
Yeah, basically that. Since it's at the build level, failures should be catastrophic and up front.
The main things that I'd look to potentially break would be things like async imported, broken asset references and such. Most of the core functionality seems to touch on these, and they seem to be working as expected.
Aside from any other concerns others may have, the only other thing I'd want to double check on is that workflow yml change. I see that there's some potential discussion to be had around this since you created a ticket, so I want to be sure that this tweak is good to go for the time being.
@keikari noticed this weird checkbox thing,
Odd. Good catch.
I took a brief look at the selectors, and I'm seeing this in prod:
vs this in the new build:
It might be a couple of days before I can get to it, but I'll dig into it and see what's resulting in the difference.
- It works fine on development build, so I'm thinking the production optimizer or new post-css thing yanked out the needed duplicate line.
- But not sure why we needed to re-define the left color for it to work correctly in the first place.
@infinite-persistence Thanks for pointing that out. Yeah, running the production build locally reproduces it, so this gives me a target to see what's up.
Btw, quick question. Does Odysee currently support right-to-left languages? I'm only asking because postcss-rtl
is no longer maintained and is not compatible with the latest version of postcss
. Not looking to try and entangle a change on that front with this change, but also a good time to trim any libraries that might happen to be unused and within arms reach.
Yup, we do support those languages. There is also an open bug for it (just fyi)
https://github.com/OdyseeTeam/odysee-frontend/issues/1229
Just addressed the checkbox issue. Also, the branch has been freshly rebased against master. So worth poking at it again.
Thank you! rebuilding on stashu now...
@infinite-persistence , I saw some sentry related changes that conflicted here and got things resolved on this side. All looks good here, but might want to double check to make sure it looks right to you as well.
Thanks for the merge and heads-up. Tried a full build with sentry auth tokens and all -- seems to be working fine.
- On an unrelated note (just thinking out loud):
-
Seems like the
vendor*
stuff have been replaced with simple hashes-
-->
-
-
But
dev:web-server
produces them, with weird "node_modules" prefix. Would be nice if dev/prod differences can be minimized, but probably not a big deal -
-
In
dev:web
, I wonder where's the generated output now ... used to be indist
like production build, so we can have quick peeks. All in-memory now?
-
Sorry for making you rebase/etc. I can try to ship this on Monday if it's all clean.
@tzarebczan No worries at all. Considering how low level it is, it's a given that this should be gone over with a fine-tooth comb. And as I mentioned to @infinite-persistence in a previous comment, it would have been totally cool for you guys to decline this PR and just use it as a reference for later work. Whatever works for your timeline works for me.
@tzarebczan just wanted to double check here to make sure there are no other todos you'd like on this one.
All good from our end, just wanted to get the creator memberships stuff shipped this week before giving this a final whirl + prod deployment. I can take care of the conflicts then.