Skip to content

Commit f07f097

Browse files
committed
Add bundle conventions to docs
1 parent 486be4b commit f07f097

File tree

8 files changed

+343
-28
lines changed

8 files changed

+343
-28
lines changed

docs/.vitepress/config.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Vitepress config
22
import { defineConfig, type UserConfig } from 'vitepress';
3+
import { groupIconMdPlugin, groupIconVitePlugin } from 'vitepress-plugin-group-icons';
34
import { withMermaid } from 'vitepress-plugin-mermaid';
45
import { withSidebar } from 'vitepress-sidebar';
56
import type { VitePressSidebarOptions } from 'vitepress-sidebar/types';
@@ -12,6 +13,11 @@ const vitepressOptions: UserConfig = {
1213
head: [['link', { rel: 'icon', href: '/devdocs/favicon.ico' }]],
1314
ignoreDeadLinks: 'localhostLinks',
1415
lastUpdated: true,
16+
markdown: {
17+
config(md) {
18+
md.use(groupIconMdPlugin);
19+
}
20+
},
1521
outDir: `${import.meta.dirname}/../../build/devdocs`,
1622
srcDir: 'src',
1723
title: 'Modules Developer Documentation',
@@ -78,6 +84,10 @@ const vitepressOptions: UserConfig = {
7884
provider: 'local'
7985
}
8086
},
87+
vite: {
88+
// @ts-expect-error something weird going on here
89+
plugins: [groupIconVitePlugin()]
90+
}
8191
};
8292

8393
const commonSideBarOptions: VitePressSidebarOptions = {

docs/.vitepress/theme/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import Theme from 'vitepress/theme';
2+
import 'virtual:group-icons.css';
3+
4+
export default Theme;

docs/package.json

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,18 @@
44
"private": true,
55
"version": "1.0.0",
66
"type": "module",
7-
"devDependencies": {
8-
"@sourceacademy/modules-lib": "workspace:^",
9-
"mermaid": "^11.7.0",
10-
"vitepress": "^1.6.3",
11-
"vitepress-plugin-mermaid": "^2.0.17",
12-
"vitepress-sidebar": "^1.31.1"
13-
},
147
"scripts": {
158
"build": "yarn build-lib-docs && vitepress build .",
169
"build-lib-docs": "yarn workspaces foreach -A --include \"@sourceacademy/modules-lib\" run docs",
1710
"dev": "vitepress dev .",
1811
"preview": "vitepress preview ."
12+
},
13+
"dependencies": {
14+
"@sourceacademy/modules-lib": "workspace:^",
15+
"mermaid": "^11.7.0",
16+
"vitepress": "^1.6.3",
17+
"vitepress-plugin-group-icons": "^1.6.1",
18+
"vitepress-plugin-mermaid": "^2.0.17",
19+
"vitepress-sidebar": "^1.31.1"
1920
}
2021
}

docs/src/modules/2-bundle/1-overview/1-overview.md

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -174,24 +174,8 @@ The helpful comment below the import explains the discrepancy. However, without
174174
![](./mqtt-types.png)
175175

176176
`tsconfig` does provide a way for you to tell Typescript where to look for types: using either the `paths` or `types` field. The `tsconfig` for the `communication` bundle looks like this:
177-
```jsonc
178-
{
179-
"extends": "../tsconfig.json",
180-
"include": [
181-
"./src"
182-
],
183-
"compilerOptions": {
184-
"outDir": "./dist",
185-
"paths": { // [!code focus]
186-
"mqtt/dist/*": ["../../../node_modules/mqtt/types"], // [!code focus]
187-
"js-slang/context": ["../../../lib/modules-lib/src/types/js-slang/context.d.ts"]// [!code focus]
188-
}// [!code focus]
189-
},
190-
"typedocOptions": {
191-
"name": "communication"
192-
}
193-
}
194-
```
177+
178+
<<< ../../../../../src/bundles/communication/tsconfig.json
195179

196180
The `paths` compiler option has been set to tell Typescript where to find the types for `mqtt/dist/mqtt`. Because of the way `tsconfig.json` file inheritance works, if you override a configuration option in a child `tsconfig`,
197181
you'll need to specify the options from the parent configuration again (hence the `js-slang/context` path).

docs/src/modules/2-bundle/3-editing.md

Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,284 @@ This adds the dependency to `devDependencies` instead.
3838
> yarn add @sourceacademy/modules-lib@workspace:^
3939
> ```
4040
41+
## Bundle Conventions
42+
To ensure that bundles conform to the different Source language specifications, there are a few rules that bundles need to abide by:
43+
44+
### 1. Cadet facing functions should not have default or rest parameters
45+
The function signature below takes in two booleans, the second of which is optional. This is discouraged in Source, but is fine if your function
46+
isn't being exposed to cadets.
47+
```ts
48+
// Don't expose this to cadets!
49+
function configure_options(option_1: boolean, option_2: boolean = false) {
50+
// ...implementation
51+
}
52+
53+
// or this
54+
function concat_strings(...args: string[]) {
55+
return args.join(',')
56+
}
57+
58+
// But default and rest parameters are okay for internal use
59+
export function exposed_function() {
60+
configure_options(true);
61+
concat_strings('str1', 'str2');
62+
}
63+
```
64+
65+
::: details Integration with `js-slang`
66+
Neither default nor rest parameters are currently supported due to an [issue](https://github.com/source-academy/js-slang/issues/1238) on the `js-slang` side.
67+
:::
68+
69+
### 2. If your bundle requires specific Source features, make sure to indicate it in the manifest
70+
Consider the bundle function below:
71+
```ts
72+
export function sum(args: numbers[]) {
73+
return args.reduce((res, each) => res + each, 0);
74+
}
75+
```
76+
It takes an an input an array of `number`s. However, arrays are only available in Source 3 onward. This means that you should indicate in your bundle's manifest:
77+
```jsonc
78+
{
79+
"requires": 3
80+
}
81+
```
82+
to let `js-slang` know that your bundle can't be loaded with Source 1 and 2.
83+
84+
::: details Which data type to use?
85+
In the above example, `args` can actually be replaced with a Source `List`:
86+
```ts
87+
import { head, tail } from 'js-slang/dist/stdlib/list';
88+
89+
export function sum(args: List) {
90+
let total = 0;
91+
while (!is_null(args)) {
92+
const value = head(args);
93+
if (typeof value !== number) {
94+
throw new Error('Expected a list of numbers!');
95+
}
96+
total += value;
97+
args = tail(args);
98+
}
99+
100+
return total;
101+
}
102+
```
103+
Lists are actually introduced in Source 1, which would make the above function compatible with Source 1 instead of requiring Source 3. If your bundle doesn't need
104+
functionality specific to arrays, then consider using Source Lists instead.
105+
:::
106+
107+
### 3. Cadet facing functions should not require object literals
108+
Object literals are not supported in Source, but might be required in bundle code. For example, in the case where your bundle might have several configurable options that the cadet can change,
109+
you should have a function for each option rather than a single function that takes all the options:
110+
```ts
111+
// Do this!
112+
export function change_text_color(color: string): void;
113+
export function change_text_size(size: number): void;
114+
115+
// And not this!
116+
interface TextOptions {
117+
color: string
118+
size: number
119+
}
120+
export function change_text_options(options: TextOptions): void
121+
```
122+
If your bundle requires the passing of objects around, they should be given an appropriate abstraction and follow the rules set out in the [next](#4-object-literalsnon-source-primitives-returned-by-bundles-should-implement-replresult) section.
123+
124+
### 4. Object literals/Non-Source Primitives returned by bundles should implement `ReplResult`
125+
If your bundle exports an object that is intended to be an abstraction, then it should implement the `ReplResult` interface.
126+
127+
For example, the `curve` bundle's `draw_connected` functions return a `RenderFunction`. The type `RenderFunction` is really just a plain Javascript function with some extra properties attached to it:
128+
129+
```ts
130+
type RenderFunction = {
131+
(func: Curve): CurveDrawn
132+
is3D: boolean
133+
}
134+
135+
// Equivalent to
136+
// type RenderFunction = ((func: Curve) => CurveDraw) & { is3D: boolean }
137+
```
138+
139+
Calling `stringify()` or `display()` (both Source builtins) on the return type of `draw_connected` would then result in the entire function body being displayed:
140+
```js
141+
curve => {
142+
// Partial toString() representation of a RenderFunction
143+
const curveDrawn = generateCurve(scaleMode, drawMode, numPoints, curve, space, isFullView);
144+
if (!curve.shouldNotAppend) {
145+
drawnCurves.push(curveDrawn);
146+
}
147+
return curveDrawn;
148+
}
149+
```
150+
This is undesirable: implementation details should be hidden from cadets. Instead, by either directly or indirectly implementing the `ReplResult` interface,
151+
a user-friendly stringified representation of the object is provided:
152+
```js
153+
import { draw_connected } from 'curve';
154+
display(draw_connected(200));
155+
156+
// Produces the output below
157+
// <RenderFunction(200)>
158+
```
159+
160+
In this case, `ReplResult` is implemented indirectly by setting the `toString` property on the `RenderFunction` before it is returned:
161+
```ts
162+
// curve/src/functions.ts
163+
164+
const func = (curve: Curve) => {
165+
const curveDrawn = generateCurve(
166+
scaleMode,
167+
drawMode,
168+
numPoints,
169+
curve,
170+
space,
171+
isFullView
172+
);
173+
174+
if (!curve.shouldNotAppend) {
175+
drawnCurves.push(curveDrawn);
176+
}
177+
178+
return curveDrawn;
179+
};
180+
// Because the draw functions are actually functions
181+
// we need hacky workarounds like these to pass information around
182+
func.is3D = space === '3D';
183+
func.toString = () => `<${space==='3D' ? '3D' : ''}RenderFunction(${numPoints})>`; // [!code highlight]
184+
return func;
185+
```
186+
187+
The `curve` bundle also has a `Point` type, which is an abstraction of a point in 3D space with a color value:
188+
```ts
189+
/** Encapsulates 3D point with RGB values. */
190+
export class Point implements ReplResult {
191+
constructor(
192+
public readonly x: number,
193+
public readonly y: number,
194+
public readonly z: number,
195+
public readonly color: Color
196+
) {}
197+
198+
public toReplString = () => `(${this.x}, ${this.y}, ${this.z}, Color: ${this.color})`;
199+
}
200+
```
201+
202+
Since `Point` is a class, it can directly implement the `ReplResult` interface. If it didn't implement this interface, then calling `display(make_point(20, 20))` would
203+
result in the infamous `[object Object]` being printed.
204+
205+
::: details `toString` vs `toReplString`
206+
For objects that are direct members of a bundle, you should use `toReplString` to provide the string representation for that object. Because `RenderFunctions` are
207+
created directly in Source code (it is `draw_connected` that is a direct member of the `curve` bundle), Source uses the `toString` property instead to obtain the string representation.
208+
:::
209+
210+
This same principle applies to other Javascript primitives as well. For example, Symbols and RegExp expressions are primitives in regular Javascript, but are not allowed in Source.
211+
212+
You can use these type abstractions directly in your functions:
213+
```ts
214+
export function make_point(x: number, y: number): Point // Returns a Point!
215+
```
216+
217+
These type abstractions do **not** need to be exported from your bundle's entry point. The documentation generators will ignore such type exports.
218+
219+
> [!TIP] Simple Abstractions
220+
> Take the type `Sound` from the `sound` bundle. A `Sound` is defined as a pair consisting where the head is a function that returns the sound's waveform
221+
> and the tail is the sound's duration:
222+
> ```ts
223+
> type Wave = (t: number) => number
224+
> type Sound = Pair<Wave, number>
225+
> ```
226+
> The `Wave` type is intentionally defined as a function that takes in a number and returns another number. For both of these types,
227+
> the default `toString` behaviour closely follows their definitions
228+
> ```ts
229+
> const s = make_sound(t => 0, 1000);
230+
> display(s);
231+
> // Produces the output below
232+
> // [t => t >= duration ? 0 : wave(t), 100]
233+
> ```
234+
> In this case, then, it becomes unnecessary to apply abstractions and implement the `ReplResult` interface.
235+
236+
### 5. Error Handling
237+
Specific to error handling, thrown errors should contain a reference to the calling function's name (using the `name` property):
238+
```ts
239+
export function make_sound(wave: Wave, duration: number): Sound {
240+
if (duration < 0) {
241+
throw new Error(`${make_sound.name}: Sound duration must be greater than or equal to 0`);
242+
}
243+
244+
return pair((t: number) => (t >= duration ? 0 : wave(t)), duration);
245+
}
246+
```
247+
248+
This helps prevent the stack trace from going deeper into internal bundle implementations, which would only serve to confuse a cadet and break abstractions.
249+
If the error is thrown from a bundle internal function, then that function should take a `name` string parameter instead:
250+
```ts
251+
// throwIfNotRune isn't supposed to be exported, but it is supposed to be called
252+
// from functions that are
253+
function throwIfNotRune(func_name: string, obj: unknown): asserts obj is Rune {
254+
if (!(rune instanceof Rune)) throw new Error(`${func_name} expects a rune as argument.`);
255+
}
256+
257+
// like show
258+
export function show(rune: Rune) {
259+
throwIfNotRune(show.name, rune);
260+
drawnRunes.push(new NormalRune(rune));
261+
return rune;
262+
}
263+
```
264+
265+
Then, the error can be thrown with the correct function name. Otherwise, cadets would see that the error originated from `throwIfNotRune`, which is not a function
266+
that is visible to them, and it doesn't tell them which function the error was thrown from (was it `show`? or `anaglyph`? or something else?)
267+
268+
#### Type Checking
269+
Though bundles are written in Typescript, Source (except for the Typed Variant) does not support anything beyond rudimentary type checking. This means that it can determine that an expression
270+
like `1 - "string"` is badly typed, but it can't type check more complex programs like the one below, especially when bundle functions are involved:
271+
272+
```ts
273+
import { show } from 'rune';
274+
275+
// Error: show expects a rune!
276+
show(1);
277+
```
278+
The above call to `show` won't a throw compile-time error. Instead, the error is detected at runtime by bundle code. This is the case even if the function has been annotated with
279+
Typescript types:
280+
```ts
281+
function throwIfNotRune(func_name: string, obj: unknown): asserts obj is Rune {
282+
if (!(rune instanceof Rune)) throw new Error(`${func_name} expects a rune as argument.`);
283+
}
284+
285+
export function show(rune: Rune) {
286+
throwIfNotRune(show.name, rune);
287+
drawnRunes.push(new NormalRune(rune));
288+
return rune;
289+
}
290+
```
291+
292+
> [!TIP]
293+
> The `asserts obj is Rune` syntax is known as a _type guard_, a feature supported by Typescript to help assist the compiler in type narrowing.
294+
> More information can be found [here](https://www.typescriptlang.org/docs/handbook/2/narrowing.html).
295+
>
296+
> Where possible, you should make use of type guards to help increase the type safety of your bundle code
297+
298+
### 6. Making use of `js-slang/stdlib`
299+
Bundles, where necessary, should use the implementations of libraries such as `list` or `stream` from the `js-slang` standard library:
300+
301+
```ts
302+
import { is_pair } from 'js-slang/dist/stdlib/list';
303+
304+
export function is_sound(obj: unknown): obj is Sound {
305+
return (
306+
is_pair(x)
307+
&& typeof get_wave(x) === 'function'
308+
&& typeof get_duration(x) === 'number'
309+
);
310+
}
311+
```
312+
These libraries get externalized and are then provided to bundles at runtime, so not only does this make your bundle size smaller, but it also
313+
ensures that you are using the same version of the `stdlib` as the version being used by `js-slang` while running your bundle code.
314+
315+
::: details An extra copy of `list`?
316+
Once again, if you've been around long enough, you might remember a time where bundles each had to provide their own copy of the `list` library.
317+
This is no longer necessary.
318+
:::
319+
41320
## Adding Unit Tests
42321
Where possible, you should add unit tests to your bundle. Refer to [this](/modules/4-advanced/testing) page for instructions.

docs/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
"resolveJsonModule": true,
77
"verbatimModuleSyntax": true
88
},
9-
"include": ["./.vitepress/config.ts", "src"]
9+
"include": ["./.vitepress/**/*.ts", "src"]
1010
}

eslint.config.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ export default tseslint.config(
137137
'@stylistic/brace-style': ['warn', '1tbs', { allowSingleLine: true }],
138138
'@stylistic/function-call-spacing': ['warn', 'never'],
139139
'@stylistic/function-paren-newline': ['warn', 'multiline-arguments'],
140+
'@stylistic/nonblock-statement-body-position': ['error', 'beside'],
141+
'@stylistic/object-curly-newline': ['warn', {
142+
ImportDeclaration: { multiline: true },
143+
}],
140144
'@stylistic/quotes': ['warn', 'single', { avoidEscape: true }],
141145
'@stylistic/semi': ['warn', 'always'],
142146
}
@@ -277,7 +281,7 @@ export default tseslint.config(
277281
name: 'Rules for tests',
278282
extends: [vitestPlugin.configs.recommended],
279283
plugins: {
280-
'vitest': vitestPlugin,
284+
vitest: vitestPlugin,
281285
},
282286
files: [
283287
'**/__tests__/**/*.ts*',

0 commit comments

Comments
 (0)