-
-
Notifications
You must be signed in to change notification settings - Fork 615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Attempt to modernize app build system #115
base: v3
Are you sure you want to change the base?
Conversation
I'm trying to modernize a bit `mediasoup-demo/app` (the browser app). It's being terrible: - AFAIU `gulp` is kinda deprecated everywhere. - I'm being forced to add `type: 'module'` in `package.json` to use `import` instead of require in `gulpfile.js`, however I need to do hacks like `import { default as rename } from 'gulp-rename';` everywhere. - Coudln't update to latest React 18 due to too many breaking changes so I just upgraded to React 16. - Hard to figure out error when running `gulp live`: ``` Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports. Check the render method of `Room`. in Room (created by Connect(Room)) in Connect(Room) (created by Context.Consumer) in Unknown in Provider ``` **I need help**
Help requested in the forum: https://mediasoup.discourse.group/t/need-help-to-modernize-mediasou-demo-build-system/5233 |
@ibc In order not to have vulnerabilities in production, I'm seeing that the
We can move Can we remove |
I will leave face-api in as I'm able to run Edit: Strike that, I got your error. Working on it. Edit 2: I've started refactoring to react 18. At least I have different errors now! |
@garrettboone definitely we can get rid of face-api stuff since it's not WebRTC related at all. Not sure why I added it. Regarding Thanks a lot. |
We can do it in another future PR, but can we remove |
mediasoup-demo doesn't use socket.io ¯_(ツ)_/¯ |
Ouch! Then it's only EduMeet, sorry! 😅 |
It seems the face-api might be used simply to recognize when a face has appeared. I haven't gotten past all the errors yet but I left it in and am doing overrides on dependencies for the problematic tensorflow libraries. Not sure what that will do but that's one of the surprises to come. |
@ibc UPDATE: The face-api started throwing an error related to backend storage for tensorflow...I think another dependency is probably needed but I've set it aside as it's I'm pretty well into the thick of refactoring and have most of the index, RoomContext, Room, etc refactored and am working through the different subcomponents. I have NetworkThrottle done and working. Even though there is more to do, at the moment when the demo runs it just gets a new room generated in a loop, no errors, and RoomClient info is getting passed to Room without any problems. I'm changing class-based components to function-based and making syntax with braces consistent, as a I go. A lot of content in the index was moved to a new RoomContextComponent. I estimate I might be able to push a test example within the next two weeks. (If I'm lucky with no other emergencies, as I'm getting to this on the side.) |
Why the change to function-based components? |
I know that using function based components is the prefered way to do it now instead of class based ones, but my question is, why to do the migration? Function based components has a lot of polemique and there's a lot of developers (including me) that think is a footshoot for React ecosystem, both in maintenance and performance... so why? |
Because I look ahead, am the one doing the work, and am personally decisive. I have no other reasons and don't feel strongly about it. |
Thanks for this effort. I'm too busy at work these days and will be a week off in next weeks but will come back to this when possible. BTW whatever approach is ok for me. Only "requirement" is that we have something to launch web app with different query params as we do today with gulp (see gulpfile.js). |
@ibc I'm continuing where you left off and am using the gulpfile.js with the changes you made. |
Nice. Thanks for this. I'll be off for some days but will come back to this. |
@garrettboone I've added this commit in master: c288a64 |
Still in progress... handling primary business issues at the moment. |
Thanks a lot. |
@ibc I need to hand this back, sorry - I've been unable to make more progress for some time due to other obligations and cannot predict "clear skies" at the moment. I can provide a link to the unfinished work if desired, though it seemed some in the community had other thoughts about the direction to go on certain things. |
Don't worry @garrettboone. If you wish you can create a draft PR here so we or others can contribute to it. |
I'm trying to modernize a bit
mediasoup-demo/app
(the browser app). It's being terrible:AFAIU
gulp
is kinda deprecated everywhere.I'm being forced to add
type: 'module'
inpackage.json
to useimport
instead of require ingulpfile.js
, however I need to do hacks likeimport { default as rename } from 'gulp-rename';
everywhere.Coudln't update to latest React 18 due to too many breaking changes so I just upgraded to React 16.
Hard to figure out error when running
gulp live
: ``` Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.Check the render method of
Room
. in Room (created by Connect(Room)) in Connect(Room) (created by Context.Consumer) in Unknown in Provider ```I need help