Skip to content

AE-87 [docs] Frontend development with ESM#1547

Open
abgreeve wants to merge 1 commit intomoodle:mainfrom
abgreeve:AE-87-main-2
Open

AE-87 [docs] Frontend development with ESM#1547
abgreeve wants to merge 1 commit intomoodle:mainfrom
abgreeve:AE-87-main-2

Conversation

@abgreeve
Copy link
Contributor

This provides some guidance for frontend development in Moodle going forward with our ESM system.

Copilot AI review requested due to automatic review settings March 26, 2026 02:40
@netlify
Copy link

netlify bot commented Mar 26, 2026

Deploy Preview for moodledevdocs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit c37b927
🔍 Latest deploy log https://app.netlify.com/projects/moodledevdocs/deploys/69c49ca69ee33f000882e936
😎 Deploy Preview https://deploy-preview-1547--moodledevdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new developer guide page describing the recommended approach for Moodle frontend development with ESM, React, and TypeScript, including templated mounting, props guidance, data-fetching patterns, and theming considerations.

Changes:

  • Introduces a new “Frontend Development” guide under docs/guides/frontend/.
  • Documents React auto-initialisation via the Mustache {{#react}} helper and the default-export contract.
  • Provides guidance on props minimisation, service-module data fetching, and theming/styling compatibility.

Comment on lines +126 to +127
"teachers": [...],
"activities": [...]
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

This block is labeled as JSON but the example contains ... placeholders (e.g. "teachers": [...]) which makes it invalid JSON. Either make it valid JSON (using explicit arrays/objects) or switch the fence to something like js/javascript and call out that it’s pseudocode.

Suggested change
"teachers": [...],
"activities": [...]
"teachers": [
{
"id": 1,
"fullname": "Dr. Smith"
}
],
"activities": [
{
"id": 10,
"name": "Assignment 1",
"type": "assignment"
}
]

Copilot uses AI. Check for mistakes.

Where ESM component wrappers are available, developers should prefer them.

Where they are not avaialable, AMD files may be accessed using compatibility helpers.
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Typo: "avaialable" should be "available".

Suggested change
Where they are not avaialable, AMD files may be accessed using compatibility helpers.
Where they are not available, AMD files may be accessed using compatibility helpers.

Copilot uses AI. Check for mistakes.

### Recommended pattern for Data Fetching

React components should avoid directly embedding data fetching lgic where possible. Instead, data should be handled through service modules.
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Typo: "lgic" should be "logic".

Suggested change
React components should avoid directly embedding data fetching lgic where possible. Instead, data should be handled through service modules.
React components should avoid directly embedding data fetching logic where possible. Instead, data should be handled through service modules.

Copilot uses AI. Check for mistakes.
Comment on lines +193 to +199
export const fetchCourse = async (courseid: number) => {
const [result] = await Ajax.call([{
methodname: 'core_course_get_courses',
args: {ids: [courseid]}
}]);

return result[0];
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The await Ajax.call([...]) example is likely incorrect for Moodle’s core/ajax API: call() returns an array of Promises (one per request), so awaiting the array won’t resolve the underlying request and result will be a Promise rather than the response. Update the example to use the documented pattern (e.g. call([...])[0] and await that Promise, or await Promise.all(call([...]))) and consider using the same import style as docs/guides/javascript/ajax/index.md (import {call as fetchMany} from 'core/ajax';).

Suggested change
export const fetchCourse = async (courseid: number) => {
const [result] = await Ajax.call([{
methodname: 'core_course_get_courses',
args: {ids: [courseid]}
}]);
return result[0];
import {call as fetchMany} from 'core/ajax';
export const fetchCourse = async (courseid: number) => {
const [request] = fetchMany([{
methodname: 'core_course_get_courses',
args: {ids: [courseid]}
}]);
const courses = await request;
return courses[0];

Copilot uses AI. Check for mistakes.
Comment on lines +217 to +222
const [loading, setLoading] = useState(null);

useEffect(() => {
fetchCourse(courseid)
.then(setCourse)
.finally(() => setLoading(false));
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

In this example loading is treated like a boolean (setLoading(false)), but it’s initialised with useState(null) and never set to true when the request starts. That makes the snippet harder to follow and doesn’t demonstrate the “loading and error states” bullet below; consider initialising loading as true/false consistently and showing where it is set at request start and on error.

Suggested change
const [loading, setLoading] = useState(null);
useEffect(() => {
fetchCourse(courseid)
.then(setCourse)
.finally(() => setLoading(false));
const [loading, setLoading] = useState(false);
const [error, setError] = useState<Error | null>(null);
useEffect(() => {
setLoading(true);
setError(null);
fetchCourse(courseid)
.then(setCourse)
.catch(setError)
.finally(() => setLoading(false));

Copilot uses AI. Check for mistakes.

### Relationship to Reactive UI System

Moodle previously introduced a custom reactive UI system to support dynamic interfaces (link to other page.
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

This sentence contains a placeholder and unmatched parenthesis: "(link to other page.". It should be replaced with a real link (likely to the Reactive UI guide under docs/guides/javascript/reactive/) and the punctuation fixed so the paragraph renders correctly.

Suggested change
Moodle previously introduced a custom reactive UI system to support dynamic interfaces (link to other page.
Moodle previously introduced a custom reactive UI system to support dynamic interfaces; see the [Reactive UI guide](../javascript/reactive/).

Copilot uses AI. Check for mistakes.

In practice, this means exporting a function that returns JSX:

```TypeScript
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Code fence language identifiers here (and elsewhere in this page) use non-standard values like TypeScript/JSON. Elsewhere in the docs (e.g. docs/guides/javascript/react/profiler.md and docs/guides/javascript/react/reactautoinit.md) the convention is to use lower-case Prism ids like ts/tsx and json, which improves consistent syntax highlighting.

Suggested change
```TypeScript
```ts

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

This should be lowercase typescript, not ts.

tsx is also supported for react tsx.


- hard coded colours
- hard coded spacing
- custom styles that duplicates the design system components
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Grammar/typo: "custom styles that duplicates" should be "custom styles that duplicate".

Suggested change
- custom styles that duplicates the design system components
- custom styles that duplicate the design system components

Copilot uses AI. Check for mistakes.
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