-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add an option to develop in a reproducible and containerized environment #10794
Conversation
Use already available webpack for development purposes only
Apparently 'scripts' and 'package.json' are not enough as mentioned in the docs and there is something hidden. Webpack errors: ERROR in Entry module not found: Error: Can't resolve './src' in '/src' ERROR in multi (webpack)-dev-server/client?http://0.0.0.0 ./src
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking in the right direction - thank you! Most of the concerns here are slightly awkward workflow constraints on our end, but should hopefully be achievable.
If you run into problems, visit us in #riot-web:matrix.org (which I think you've found?).
@@ -0,0 +1,45 @@ | |||
FROM node:10-alpine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
. o ( We should probably bump this to 11 or 12 soon. Something for a different PR )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely need to bump it to node-14 now, as we don't support anything older than that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use node:lts-alpine
if you want
# Cache package.json | ||
COPY package.json yarn.lock /src/ | ||
|
||
# Remove 'prepare' script because there is NO config in yarn and npm for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meh - this is fine tbh. It shouldn't cause problems, it's just a waste of cycles.
tbh I'm more inclined to leave it there than rip it out for future maintenance things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried removing it again today and it throws this error:
Step 12/15 : RUN yarn --network-timeout=100000 install
---> Running in a026b8e776e8
yarn install v1.17.3
[1/4] Resolving packages...
[2/4] Fetching packages...
warning Pattern ["gemini-scrollbar@matrix-org/gemini-scrollbar#b302279"] is trying to unpack in the same destination "/usr/local/share/.cache/yarn/v4/npm-gemini-scrollbar-1.4.3/node_modules/gemini-scrollbar" as pattern ["gemini-scrollbar@github:matrix-org/gemini-scrollbar#b302279"]. This could result in non-deterministic behavior, skipping.
info [email protected]: The platform "linux" is incompatible with this module.
info "[email protected]" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
warning "matrix-react-sdk > [email protected]" has unmet peer dependency "emojibase@*".
warning "matrix-react-sdk > [email protected]" has unmet peer dependency "immutable@>=3.8.1".
warning "matrix-react-sdk > [email protected]" has unmet peer dependency "immutable@>=3.8.1".
warning "matrix-react-sdk > [email protected]" has unmet peer dependency "immutable@>=3.0.0".
warning "matrix-react-sdk > [email protected]" has unmet peer dependency "immutable@>=3.8.1".
warning "matrix-react-sdk > diff-dom > [email protected]" has unmet peer dependency "rollup@>=0.66.0 <2".
warning "matrix-react-sdk > slate-react > [email protected]" has unmet peer dependency "immutable@>=3.6.2".
warning "matrix-react-sdk > slate-react > [email protected]" has unmet peer dependency "immutable@>=3.8.1".
warning "matrix-react-sdk > slate-react > [email protected]" has unmet peer dependency "immutable@>=3.8.1".
warning " > [email protected]" has unmet peer dependency "app-builder-lib@~21.2.0".
[4/4] Building fresh packages...
$ yarn clean && yarn build:compile
yarn run v1.17.3
$ rimraf lib webapp electron_app/dist
Done in 0.24s.
yarn run v1.17.3
$ yarn reskindex && babel --source-maps -d lib src
$ reskindex -h src/header
src doesn't exist
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
The command '/bin/sh -c yarn --network-timeout=100000 install' returned a non-zero code: 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should fix that error then - that file definitely exists. Is it just not making its way over to the container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eh, this is embarrassing... for some reason it never occurred to me it's a file ^_^" Fixed!
# Support custom branches of the react-sdk and js-sdk. This also helps us build | ||
# images of riot-web develop. | ||
ARG USE_CUSTOM_SDKS=false | ||
ARG REACT_SDK_REPO="https://github.com/matrix-org/matrix-react-sdk.git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should actually be using locally-linked react-sdk and such when doing development, as the bulk of Riot is in the react-sdk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the variable is still here :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is, otherwise the container would require people to mount files to certain location. Currently it by default pulls the repos, however allows to override this behavior. This way not only you can choose what repo/folder to use, but also allows you to have only this project as editable without even pulling the SDKs. See this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for a development environment where 95% of the app is in the react-sdk, I would definitely expect that people have a local copy of the react-sdk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current approach works for both cases and at the same time pulls the app dependencies first which means that there already are dependencies for react-sdk and riot-web already cached in the image itself. That means a user that mounts a locally cloned react-sdk does not have to wait long for the installation because the only thing that's going to install with docker run
is riot-web + different versions of deps that are not cached. This is good for switching between branches since docker container by default does not save the changes in its filesystem(image) unless docker commit
is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The installation should be pretty quick if the dependencies have already been pulled the first time. Unfortunately the environment doesn't provide much value if most of the app isn't editable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docker does not work that way, it does not preserve anything that happens in the container which means every docker run
command requires the installation from scratch i.e. pulling each dependency. No quick install because the pull would happen every time. That's why I left the original repositories there so the docker image already contains the cached dependencies. Only image preserves the state, not container. If I remove the repositories from Dockerfile, everything will be slowed down because the whole fetching/hashing/installing/linking/preparing process would need to be executed in full on each docker run
. Currently if you do docker build
the image (which contains the state used by the container) will have all the deps + a default installation according to that ARGS instruction, however you don't need to care about that because as said in the linked instructions from README.md with docker run
you can specify to use custom, locally available, repositories/folders which will then replace the default installation from the image and will be both used for the next installation run with docker run
command (installs only the new SDKs, uses cached deps, i.e. the case you describe) and will be editable from the host machine i.e. if I change something in react-sdk and press F5 the changes will be immediately reflected in the container. If anything goes wrong a user can then use docker commit
to create a new image from the container and e.g. send it to you/other devs or just share with colleagues.
README.md
Outdated
# such as res with additional --volume $PWD/res:/src/res | ||
docker run --detach --interactive --tty \ | ||
--publish <port-on-host>:8080 \ | ||
--volume $PWD/src:/src/src \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/riot/src
would still be better, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working on it! :) Hopefully until the end of the week I'll have more time for testing Docker builds, since this will need to change the folder within Dockerfile
(already present in master
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
This will allow to link external matrix-react-sdk/matrix-js-sdk from host machine after the long installation process has been completed and re-launch the installation with the only deps changed being Matrix SDKs producing a quick installation.
@turt2live I've implemented two ways for using a custom SDKs, one in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're much closer now, thank you! Just a couple things left to resolve.
@KeyWeeUsr Has this been abandoned? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so u have to describe the issue more for me thank you
@KeyWeeUsr is this still relevant? If yes, ping me and I'll take care of it. :) |
Well, afaik it still is, I haven't really seen any alternative provided. What exactly still needs to be changed in this PR though (except the obvious |
conflicts and review comments, please resolve them if you think they have been taken care of. |
I think so, to me your explanation sounds good, especially considering that the VSCode's development in docker is so much better now and it'll work much faster than sharing directory on root machine (especially on MacOS :), but just to make sure I'll ping @turt2live too. |
So we've taken another look at this and don't currently have a pressing need for it within the team, but if folks are interested in the sort of benefits this offers then please open a new issue/PR and we can discuss it there. Thanks for taking a look at this, and apologies for the slow review cycle. |
* Redirect to the SSO page if `sso_redirect_options.on_welcome_page` is enabled and the URL hash is empty ([\element-hq#25495](element-hq#25495)). Contributed by @dhenneke. * vector/index.html: Allow fetching blob urls ([\element-hq#25336](element-hq#25336)). Contributed by @SuperKenVery. * When joining room in sub-space join the parents too ([\element-hq#11011](matrix-org/matrix-react-sdk#11011)). * Include thread replies in message previews ([\element-hq#10631](matrix-org/matrix-react-sdk#10631)). Fixes element-hq#23920. * Use semantic headings in space preferences ([\element-hq#11021](matrix-org/matrix-react-sdk#11021)). Contributed by @kerryarchibald. * Use semantic headings in user settings - Ignored users ([\element-hq#11006](matrix-org/matrix-react-sdk#11006)). Contributed by @kerryarchibald. * Use semantic headings in user settings - profile ([\element-hq#10973](matrix-org/matrix-react-sdk#10973)). Fixes element-hq#25461. Contributed by @kerryarchibald. * Use semantic headings in user settings - account ([\element-hq#10972](matrix-org/matrix-react-sdk#10972)). Contributed by @kerryarchibald. * Support `Insert from iPhone or iPad` in Safari ([\element-hq#10851](matrix-org/matrix-react-sdk#10851)). Fixes element-hq#25327. Contributed by @SuperKenVery. * Specify supportedStages for User Interactive Auth ([\element-hq#10975](matrix-org/matrix-react-sdk#10975)). Fixes element-hq#19605. * Pass device id to widgets ([\element-hq#10209](matrix-org/matrix-react-sdk#10209)). Contributed by @Fox32. * Use semantic headings in user settings - discovery ([\element-hq#10838](matrix-org/matrix-react-sdk#10838)). Contributed by @kerryarchibald. * Use semantic headings in user settings - Notifications ([\element-hq#10948](matrix-org/matrix-react-sdk#10948)). Contributed by @kerryarchibald. * Use semantic headings in user settings - spellcheck and language ([\element-hq#10959](matrix-org/matrix-react-sdk#10959)). Contributed by @kerryarchibald. * Use semantic headings in user settings Appearance ([\element-hq#10827](matrix-org/matrix-react-sdk#10827)). Contributed by @kerryarchibald. * Use semantic heading in user settings Sidebar & Voip ([\element-hq#10782](matrix-org/matrix-react-sdk#10782)). Contributed by @kerryarchibald. * Use semantic headings in user settings Security ([\element-hq#10774](matrix-org/matrix-react-sdk#10774)). Contributed by @kerryarchibald. * Use semantic headings in user settings - integrations and account deletion ([\#10837](matrix-org/matrix-react-sdk#10837)). Fixes element-hq#25378. Contributed by @kerryarchibald. * Use semantic headings in user settings Preferences ([\element-hq#10794](matrix-org/matrix-react-sdk#10794)). Contributed by @kerryarchibald. * Use semantic headings in user settings Keyboard ([\element-hq#10793](matrix-org/matrix-react-sdk#10793)). Contributed by @kerryarchibald. * RTE plain text mentions as pills ([\element-hq#10852](matrix-org/matrix-react-sdk#10852)). Contributed by @alunturner. * Allow welcome.html logo to be replaced by config ([\element-hq#25339](element-hq#25339)). Fixes element-hq#8636. * Use semantic headings in user settings Labs ([\element-hq#10773](matrix-org/matrix-react-sdk#10773)). Contributed by @kerryarchibald. * Use semantic list elements for menu lists and tab lists ([\element-hq#10902](matrix-org/matrix-react-sdk#10902)). Fixes element-hq#24928. * Fix aria-required-children axe violation ([\element-hq#10900](matrix-org/matrix-react-sdk#10900)). Fixes element-hq#25342. * Enable pagination for overlay timelines ([\element-hq#10757](matrix-org/matrix-react-sdk#10757)). Fixes vector-im/voip-internal#107. * Add tooltip to disabled invite button due to lack of permissions ([\element-hq#10869](matrix-org/matrix-react-sdk#10869)). Fixes element-hq#9824. * Respect configured auth_header_logo_url for default Welcome page ([\element-hq#10870](matrix-org/matrix-react-sdk#10870)). * Specify lazy loading for avatars ([\element-hq#10866](matrix-org/matrix-react-sdk#10866)). Fixes element-hq#1983. * Room and user mentions for plain text editor ([\element-hq#10665](matrix-org/matrix-react-sdk#10665)). Contributed by @alunturner. * Add audible notifcation on broadcast error ([\#10654](matrix-org/matrix-react-sdk#10654)). Fixes element-hq#25132. * Fall back from server generated thumbnail to original image ([\element-hq#10853](matrix-org/matrix-react-sdk#10853)). * Use semantically correct elements for room sublist context menu ([\element-hq#10831](matrix-org/matrix-react-sdk#10831)). Fixes vector-im/customer-retainer#46. * Avoid calling prepareToEncrypt onKeyDown ([\element-hq#10828](matrix-org/matrix-react-sdk#10828)). * Allows search to recognize full room links ([\element-hq#8275](matrix-org/matrix-react-sdk#8275)). Contributed by @bolu-tife. * "Show rooms with unread messages first" should not be on by default for new users ([\element-hq#10820](matrix-org/matrix-react-sdk#10820)). Fixes element-hq#25304. Contributed by @kerryarchibald. * Fix emitter handler leak in ThreadView ([\element-hq#10803](matrix-org/matrix-react-sdk#10803)). * Add better error for email invites without identity server ([\element-hq#10739](matrix-org/matrix-react-sdk#10739)). Fixes element-hq#16893. * Move reaction message previews out of labs ([\element-hq#10601](matrix-org/matrix-react-sdk#10601)). Fixes element-hq#25083. * Sort muted rooms to the bottom of their section of the room list ([\element-hq#10592](matrix-org/matrix-react-sdk#10592)). Fixes element-hq#25131. Contributed by @kerryarchibald. * Use semantic headings in user settings Help & About ([\element-hq#10752](matrix-org/matrix-react-sdk#10752)). Contributed by @kerryarchibald. * use ExternalLink components for external links ([\element-hq#10758](matrix-org/matrix-react-sdk#10758)). Contributed by @kerryarchibald. * Use semantic headings in space settings ([\element-hq#10751](matrix-org/matrix-react-sdk#10751)). Contributed by @kerryarchibald. * Use semantic headings for room settings content ([\element-hq#10734](matrix-org/matrix-react-sdk#10734)). Contributed by @kerryarchibald. * Use consistent fonts for Japanese text ([\element-hq#10980](matrix-org/matrix-react-sdk#10980)). Fixes element-hq#22333 and element-hq#23899. * Fix: server picker validates unselected option ([\element-hq#11020](matrix-org/matrix-react-sdk#11020)). Fixes element-hq#25488. Contributed by @kerryarchibald. * Fix room list notification badges going missing in compact layout ([\element-hq#11022](matrix-org/matrix-react-sdk#11022)). Fixes element-hq#25372. * Fix call to `startSingleSignOn` passing enum in place of idpId ([\element-hq#10998](matrix-org/matrix-react-sdk#10998)). Fixes element-hq#24953. * Remove hover effect from user name on a DM creation UI ([\element-hq#10887](matrix-org/matrix-react-sdk#10887)). Fixes element-hq#25305. Contributed by @luixxiul. * Fix layout regression in public space invite dialog ([\element-hq#11009](matrix-org/matrix-react-sdk#11009)). Fixes element-hq#25458. * Fix layout regression in session dropdown ([\element-hq#10999](matrix-org/matrix-react-sdk#10999)). Fixes element-hq#25448. * Fix spacing regression in user settings - roles & permissions ([\element-hq#10993](matrix-org/matrix-react-sdk#10993)). Fixes element-hq#25447 and element-hq#25451. Contributed by @kerryarchibald. * Fall back to receipt timestamp if we have no event (react-sdk part) ([\element-hq#10974](matrix-org/matrix-react-sdk#10974)). Fixes element-hq#10954. Contributed by @andybalaam. * Fix: Room header 'view your device list' does not link to new session manager ([\element-hq#10979](matrix-org/matrix-react-sdk#10979)). Fixes element-hq#25440. Contributed by @kerryarchibald. * Fix display of devices without encryption support in Settings dialog ([\element-hq#10977](matrix-org/matrix-react-sdk#10977)). Fixes element-hq#25413. * Use aria descriptions instead of labels for TextWithTooltip ([\element-hq#10952](matrix-org/matrix-react-sdk#10952)). Fixes element-hq#25398. * Use grapheme-splitter instead of lodash for saving emoji from being ripped apart ([\element-hq#10976](matrix-org/matrix-react-sdk#10976)). Fixes element-hq#22196. * Fix: content overflow in settings subsection ([\#10960](matrix-org/matrix-react-sdk#10960)). Fixes element-hq#25416. Contributed by @kerryarchibald. * Make `Privacy Notice` external link on integration manager ToS clickable ([\element-hq#10914](matrix-org/matrix-react-sdk#10914)). Fixes element-hq#25384. Contributed by @luixxiul. * Ensure that open message context menus are updated when the event is sent ([\element-hq#10950](matrix-org/matrix-react-sdk#10950)). * Ensure that open sticker picker dialogs are updated when the widget configuration is updated. ([\#10945](matrix-org/matrix-react-sdk#10945)). * Fix big emoji in replies ([\element-hq#10932](matrix-org/matrix-react-sdk#10932)). Fixes element-hq#24798. * Hide empty `MessageActionBar` on message edit history dialog ([\element-hq#10447](matrix-org/matrix-react-sdk#10447)). Fixes element-hq#24903. Contributed by @luixxiul. * Fix roving tab index getting confused after dragging space order ([\element-hq#10901](matrix-org/matrix-react-sdk#10901)). * Attempt a potential workaround for stuck notifs ([\element-hq#3384](matrix-org/matrix-js-sdk#3384)). Fixes element-hq#25406. Contributed by @andybalaam. * Handle trailing dot FQDNs for domain-specific config.json files ([\element-hq#25351](element-hq#25351)). Fixes element-hq#8858. * Ignore edits in message previews when they concern messages other than latest ([\element-hq#10868](matrix-org/matrix-react-sdk#10868)). Fixes element-hq#14872. * Send correct receipts when viewing a room ([\element-hq#10864](matrix-org/matrix-react-sdk#10864)). Fixes element-hq#25196. * Fix timeline search bar being overlapped by the right panel ([\element-hq#10809](matrix-org/matrix-react-sdk#10809)). Fixes element-hq#25291. Contributed by @luixxiul. * Fix the state shown for call in rooms ([\element-hq#10833](matrix-org/matrix-react-sdk#10833)). * Add string for membership event where both displayname & avatar change ([\element-hq#10880](matrix-org/matrix-react-sdk#10880)). Fixes element-hq#18026. * Fix people space notification badge not updating for new DM invites ([\element-hq#10849](matrix-org/matrix-react-sdk#10849)). Fixes element-hq#23248. * Fix regression in emoji picker order mangling after clearing filter ([\element-hq#10854](matrix-org/matrix-react-sdk#10854)). Fixes element-hq#25323. * Fix: Edit history modal crash ([\#10834](matrix-org/matrix-react-sdk#10834)). Fixes element-hq#25309. Contributed by @kerryarchibald. * Fix long room address and name not being clipped on room info card and update `_RoomSummaryCard.pcss` ([\element-hq#10811](matrix-org/matrix-react-sdk#10811)). Fixes element-hq#25293. Contributed by @luixxiul. * Treat thumbnail upload failures as complete upload failures ([\element-hq#10829](matrix-org/matrix-react-sdk#10829)). Fixes element-hq#7069. * Update finite automata to match user identifiers as per spec ([\#10798](matrix-org/matrix-react-sdk#10798)). Fixes element-hq#25246. * Fix icon on empty notification panel ([\element-hq#10817](matrix-org/matrix-react-sdk#10817)). Fixes element-hq#25298 and element-hq#25302. Contributed by @luixxiul. * Fix: Threads button is highlighted when I create a new room ([\element-hq#10819](matrix-org/matrix-react-sdk#10819)). Fixes element-hq#25284. Contributed by @kerryarchibald. * Fix the top heading of notification panel ([\element-hq#10818](matrix-org/matrix-react-sdk#10818)). Fixes element-hq#25303. Contributed by @luixxiul. * Fix the color of the verified E2EE icon on `RoomSummaryCard` ([\element-hq#10812](matrix-org/matrix-react-sdk#10812)). Fixes element-hq#25295. Contributed by @luixxiul. * Fix: No feedback when waiting for the server on a /delete_devices request with SSO ([\element-hq#10795](matrix-org/matrix-react-sdk#10795)). Fixes element-hq#23096. Contributed by @kerryarchibald. * Fix: reveal images when image previews are disabled ([\element-hq#10781](matrix-org/matrix-react-sdk#10781)). Fixes element-hq#25271. Contributed by @kerryarchibald. * Fix accessibility issues around the room list and space panel ([\element-hq#10717](matrix-org/matrix-react-sdk#10717)). Fixes element-hq#13345. * Ensure tooltip contents is linked via aria to the target element ([\#10729](matrix-org/matrix-react-sdk#10729)). Fixes vector-im/customer-retainer#43.
Adds an option to develop this application in a containerized environment that provides reproducibility between multiple machines with the same change (branch / pull request / ...). In the less common approach it provides a way to develop applications (mainly web) without installing Node.js and maintain multiple dependencies, node_modules, etc on the host machine without an easy way to just burn everything down and start from scratch in case of a wrong approach.
Docker allows kind of
git stash
andgit stash pop
behavior with images therefore everything can be properly isolated, the changes can be done via mounting appropriate folders from host machine and in the worst case you can create an image viadocker commit
in case something went horribly wrong and you want your colleague to check it out on his/her machine (seedocker save
). Currently the image takes about 855MB on my machine, but that might be later improved.This way you can have an image for
develop
,master
,fix-12345
, etc and also work just fine on Riot X for example if it uses the same repo (not sure, haven't checked out yet). Storage nowadays isn't much of an issue I'd say, however having collisions between deps while working on multiple projects or the need to provide versions of multiple random binaries between colleagues just to reproduce why something does not work is rather inefficient.Hopefully this will help somebody else other than me :)
This PR currently has no changelog labels, so will not be included in changelogs.
A reviewer can add one of:
T-Deprecation
,T-Enhancement
,T-Defect
,T-Task
to indicate what type of change this is, or addType: [enhancement/defect/task]
to the description and I'll add them for you.