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

Add node container support #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mackensen
Copy link

This adds a node container, locked at the necessary core-supported version, for running grunt tasks. The script is based on a Stackoverflow post: https://stackoverflow.com/a/37102013. It's a work-around to ensure that the Moodle web root is loaded before the container runs the command. The expected result (verified locally) is that grunt watch is running and compiling changes. There's probably a more elegant solution to this but I wanted to share what I'd gotten working.

@@ -0,0 +1,13 @@
FROM debian:stretch-slim

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use FROM node? Also there is already a discussion about including node in the moodle-apache-php image

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FROM debian:stretch-slim
FROM node

context: ./
dockerfile: node.dockerfile
volumes:
- "${MOODLE_DOCKER_WWWROOT}:/var/www/html

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- "${MOODLE_DOCKER_WWWROOT}:/var/www/html
- "${MOODLE_DOCKER_WWWROOT}:/var/www/html"

FROM debian:stretch-slim

RUN apt-get update && apt-get install -my gnupg curl
RUN curl -sL https://deb.nodesource.com/setup_8.x | bash -
Copy link

@odeialba odeialba Jul 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RUN curl -sL https://deb.nodesource.com/setup_8.x | bash -
RUN curl -sL https://deb.nodesource.com/setup_14.x | bash -

nodejs

RUN /usr/bin/npm install -g grunt-cli
ADD ./bin/start-grunt.sh .

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ADD ./bin/start-grunt.sh .
ADD ./bin/start-grunt.sh .
USER root
RUN chmod 755 ./start-grunt.sh

@stronk7
Copy link
Member

stronk7 commented Dec 9, 2022

Hi,

as far as we include .nvmrc for all currently supported versions of Moodle (and older), wouldn't be better to include in the webserver image also nvm, so a simple nvm install & nvm use does the trick within the webserver container?

The problem I can see with this PR is that it requires us to be sticky to a given node version and that can be tricky.

Note I've not tried the idea above, maybe it has other problems, just I'm not sure if adding any extra node container really helps, hence, thinking about potential alternatives.

Also, side note: since #242 is possible to have a new local.yml file providing as many customisations as wanted. This may fall under those customisations easily, I imagine.

Thoughts about the idea (or any other idea related to ease of node/npm use) ?

Ciao :-)

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

Successfully merging this pull request may close these issues.

4 participants