Skip to content
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

fix Dockerfile #119

Merged
merged 2 commits into from
Dec 4, 2021
Merged

fix Dockerfile #119

merged 2 commits into from
Dec 4, 2021

Conversation

nosovk
Copy link
Contributor

@nosovk nosovk commented Nov 17, 2021

  • remove obsolete node-sass
  • add sass package that do not require binary deps (node-gyp_
  • remove unused deps from dockerfile
  • use LTS node version in Dockerfile
  • use npm ci instead of npm i

Description

Reference issue

Fixes #...
#117
#99
#94

Closes: #113

@nosovk
Copy link
Contributor Author

nosovk commented Nov 29, 2021

also closes #122

- add sass package that do not require binary deps (node-gyp_
- remove unused deps from dockerfile
- use LTS node version in Dockerfile
- use `npm ci` instead of npm i
@nosovk
Copy link
Contributor Author

nosovk commented Nov 29, 2021

@adwpc
could you please review\merge this PR?
We use fork with fix for some time now. It builds and starts correctly.

@gcavanunez
Copy link

Hi @nosovk on the PR #122 I was looking to essentially remove sass entirely and just use vanilla css since no extra feature from sass is being utilized in the project

@adwpc
Copy link
Contributor

adwpc commented Nov 30, 2021

@nosovk Thanks for #119 !

remove obsolete node-sass
add sass package that do not require binary deps (node-gyp_
remove unused deps from dockerfile
use LTS node version in Dockerfile
use npm ci instead of npm i

@gcavanunez Thanks for #122 !

Removing node-sass and sass-loader
Since no specially feature from sass is being utilized seems best to drop the dependency and use vanilla CSS. This should also make it feasible to install in newer versions of node without issues.

@kangshaojun is origin author, advise to just keep sass
Im not good at frontend, after some research , i think we can just keep sass.
Thank you all for any Feature/Issue/PR, i can give any help if you guys want contribute this project.

@nosovk
Copy link
Contributor Author

nosovk commented Dec 1, 2021

Then probably we can merge that PR? Or there are some change requests?

@gcavanunez gcavanunez mentioned this pull request Dec 1, 2021
@adwpc adwpc merged commit 355e704 into ionorg:master Dec 4, 2021
@adwpc
Copy link
Contributor

adwpc commented Dec 4, 2021

@nosovk got these error when use docker with chrome:
WebSocket connection to 'wss://localhost:5551/room.RoomSignal/Signal' failed:
WebSocket connection to 'wss://localhost:5551/rtc.RTC/Signal' failed:

@nosovk
Copy link
Contributor Author

nosovk commented Dec 4, 2021

@adwpc you have to launch ion backend on localhost to avoide that problem and update caddyconfig: https://github.com/pion/ion-app-web/blob/eb384c0ee580cdadcd8a20e7dfbbc9f1657d5949/prod.Caddyfile#L7

Probably we should update documentation to explain that part of config should be updated in that file during deploy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants