Improvements to the Module System 2

[leeyi45]

This PR is a follow up to #148. motivated by the idea to streamline the existing module system.

Refer to this and this for changes required to the frontend and js-slang.

Primary Change

Currently, modules are written with the following format:

// src/bundles/module/index.ts

import {
  // a bunch of things
} from './functions';

export default function module(moduleParams, moduleContexts) {
  // work with the module context
  
  return {
    // the module's exports
  }
}

New Export Syntax

Under this new PR, modules are written following the ESM syntax

// src/bundles/module/index.ts

export {
  // things to export
} from './functions';

If desired, modules can use a default export to export an object containing all its exports:

// src/bundles/module/index.ts

export default new Proxy({
  // things to export
}, {
  get(target, name) {
    if (target[name] === undefined) {
      throw new Error(`unknown import: ${name}`);
    }
    return target[name];
  }
});

// OR more simply
import {
  // things to export
} from './functions';

export default {
  // things to export
};

Mixing default and named exports will cause the default export to become hidden, so this is not recommended.

Accessing context

The context object is now available as an import:

// src/bundles/curve/functions.ts
import { context } from 'js-slang/moduleHelpers';

const curvesDrawn = [];
context.moduleContexts.curve.state = {
  curvesDrawn,
};

// other module functions

js-slang doesn't actually export the context object, but a type declaration has been added for Rollup to convert to an external parameter when the bundles get transpiled to iife.

Secondary Changes

moduleParams

The moduleParams object has been removed since it was only being used by the game module, and the functionality it requires can be achieved using module contexts.

Module Context Guarantees

In a source program, it is perfectly valid to have multiple imports on different lines to the same module so long as they are at the top level:

import { draw_connected } from 'curve';

// some user code

import { unit_circle } from 'curve';

js-slang guarantees that no matter how many import calls to a module is made, each module file is only evaluated once per code evaluation, meaning that module states are only created once per evaluation.

This means that checking if the module context and state have been initialized is unnecessary:

// Old code in curves/index.ts

// Update the module's global context
let moduleContext = moduleContexts.get('curve');

if (!moduleContext) {   
  moduleContext = {
    tabs: [],
    state: {
      drawnCurves,
    },
  };
  moduleContexts.set('curve', moduleContext);
} else if (!moduleContext.state) {
  moduleContext.state = {
    drawnCurves,
  };
} else {
  (moduleContext.state as CurveModuleState).drawnCurves = drawnCurves;
}

If your module code only initializes state information within module code, you can simply write:

// curve/functions.ts
import { context } from 'js-slang/moduleHelpers';
const drawnCurves = [];

// The curve object on moduleContexts has already been created by js-slang
context.moduleContexts.curve.state = {
  drawnCurves,
}

For modules that initialize their state object outside of module code, the state object can be used in the following manner:

// src/game/functions.ts
import { context } from 'js-slang/moduleHelpers';

const {
    scene,
    preloadImageMap,
    preloadSoundMap,
    preloadSpritesheetMap,
    remotePath,
    screenSize,
    createAward,
  } = context.moduleContexts.game.state || {};

Similarly, tabs can also follow the same guidelines:

// src/tabs/Curve/index.tsx
export default {
  toSpawn(context: DebuggerContext) {
   // If a module's bundle file is written properly, its module context should be properly initialized here
    const drawnCurves = context.context.moduleContexts.curve.state.drawnCurves;
    return drawnCurves.length > 0;
  },
  body(context: DebuggerContext) {
    // This is only called if toSpawn returns true, so can assume that this destructuring won't throw
    const { context: { moduleContexts: { curve: { state: { drawnCurves } } } } } = context;

    return <MultiItemDisplay elements={canvases} />;
  },
  label: 'Curves Tab',
  iconName: 'media', // See https://blueprintjs.com/docs/#icons for more options
};

Build Script Changes

1. If you get the error that import assertions aren't supported, you may have to update your node version to the latest LTS
2. Because of the increasing size of bundles and their files, node may actually run out of memory space when running build tasks. Either run fewer tasks at once, or modify package.json with the max-old-space-size option as necessary:

"scripts": {
    "scripts": "cross-env TS_NODE_PROJECT=\"scripts/tsconfig.json\" NODE_OPTIONS=\"--max-old-space-size=8192\" ts-node scripts/index.ts",
}

[Cloud7050]

(I edited my previous comment while you happened to be composing a reply. Please also refer to the added text.)

  body(context: DebuggerContext) {
    // This is only called if toSpawn returns true, so can assume that this destructuring won't throw
    const { context: { moduleContexts: { curve: { state: { drawnCurves } } } } } = context;

    return <MultiItemDisplay elements={canvases} />;
  },

I don't know if something has since changed, however it is worth pointing out bug source-academy/frontend#2136 where body() is called before toSpawn() is checked. If it has been fixed, then the issue can thankfully be closed.

I discovered this while working with the older version of module contexts, when developing CSG. I have a Core type that helped work with module states, and had to initialise it once on the bundle end and again on the tab end. I had to put the initialisation in body(), after which the initialised content was available to use in toSpawn(). If the bug has been fixed, I'll probably need to change that.

[Cloud7050]

This error arises if you're not using the right version of js-slang

I see, it it fitting for this PR to bump its js-slang version now that the associated Source PR has been merged?

for the new module format to work, I needed to write an output plugin for Rollup that parses the bundles and tabs into an AST and make the edits there.

It's great that the string matching is no longer hardcoded on the receiving end. When I resume work on CSG, I will monitor the build times. As much as I prefer elegant solutions like dynamically removing the function call, the speed increase may be worth taking the "dumb" approach (I don't think the matching would need frequent/complex updating). Unless there are now other edits the AST method is making?

I am not sure how to proceed with the documentation issue. The error message doesn't reveal a lot. I've reinstalled packages, and my software has all been recently updated. For reference, I'm on Windows. Side note, is one of the "file modified" logs misfiring?

...
• Csg: Build successful (1549.69 KB)

Docs Error: Failed to initialize Typedoc project
Done in 247.56s.

... >yarn run build:docs
yarn run v1.22.19
$ yarn scripts build docs
$ cross-env TS_NODE_PROJECT="scripts/tsconfig.json" NODE_OPTIONS="--max-old-space-size=8192" ts-node scripts/index.ts build docs
(node:32880) ExperimentalWarning: Importing JSON modules is an experimental feature. This feature could change at any time
(Use node --trace-warnings ... to show where the warning was created)
• File modified: src\bundles\repeat\functions.ts
Building HTML documentation

Building documentation for the following bundles:
• repeat
...
• csg
Docs Error: Failed to initialize Typedoc project
Done in 13.50s.

About ESLint, I'll probably take a look at the configs in detail down the line. If not now then I'll likely make some tweaks when I resume work on CSG.

It does not seem that Rollup is the bottleneck.

So from the earlier edit, do you think it's just a result of the extra work (AST) that rollup as a whole is now doing during the build process?

[leeyi45]

This error arises if you're not using the right version of js-slang

I see, it it fitting for this PR to bump its js-slang version now that the associated Source PR has been merged?

for the new module format to work, I needed to write an output plugin for Rollup that parses the bundles and tabs into an AST and make the edits there.

It's great that the string matching is no longer hardcoded on the receiving end. When I resume work on CSG, I will monitor the build times. As much as I prefer elegant solutions like dynamically removing the function call, the speed increase may be worth taking the "dumb" approach (I don't think the matching would need frequent/complex updating). Unless there are now other edits the AST method is making?

I am not sure how to proceed with the documentation issue. The error message doesn't reveal a lot. I've reinstalled packages, and my software has all been recently updated. For reference, I'm on Windows. Side note, is one of the "file modified" logs misfiring?

...
• Csg: Build successful (1549.69 KB)
Docs Error: Failed to initialize Typedoc project
Done in 247.56s.

... >yarn run build:docs
yarn run v1.22.19
$ yarn scripts build docs
$ cross-env TS_NODE_PROJECT="scripts/tsconfig.json" NODE_OPTIONS="--max-old-space-size=8192" ts-node scripts/index.ts build docs
(node:32880) ExperimentalWarning: Importing JSON modules is an experimental feature. This feature could change at any time
(Use node --trace-warnings ... to show where the warning was created)
• File modified: src\bundles\repeat\functions.ts
Building HTML documentation
Building documentation for the following bundles:
• repeat
...
• csg
Docs Error: Failed to initialize Typedoc project
Done in 13.50s.

About ESLint, I'll probably take a look at the configs in detail down the line. If not now then I'll likely make some tweaks when I resume work on CSG.

It does not seem that Rollup is the bottleneck.

So from the earlier edit, do you think it's just a result of the extra work (AST) that rollup as a whole is now doing during the build process?

It wasn't the AST, actually that part is almost inconsequential in my testing. I'll go do some benchmarking tests.

I usually build this stuff using my M1 MBP which takes me 2 mins tops with documentation, and that felt reasonable to me, so I never thought to try on another machine. Lesson learned, will definitely keep this in mind in the future.

[leeyi45]

Building all bundles and tabs at the same time yielded the following timings on my Windows machine:

I used rollup-timer to time plugins, but the package hasn't been updated in a while and returns some crazy results. These are the timings of transpiling the Curve tab:

[martin-henz]

Let's aim for merging and deploying for September 29. There is a week in CS1101S when modules are not used.
Till then, let's put this in "draft" mode.

[Cloud7050]

I ran a few no-documentation builds on the new version. This was the fastest one:

The new throw from buildDocsAndJsons() means the full build script (with documentation) exits early and doesn't reach timings logging for me.

For comparison, I ran one full build on the previous rollup build system (around current master 46c7513). It took just 89.21s with documentation, compared to this system's best time of 232.93s without documentation, which I find is a highly significant difference. It was run on an overclocked i9-9900K, 32GB RAM, SSD. Maybe the difference in Mac vs Windows timings can be chalked up to Apple silicon, but when comparing the previous vs new systems on the same platform, I feel the difference warrants further investigation.

---

I noticed that edits to the common/ tab Components alone aren't picked up as changes, probably because they aren't standalone tabs used in modules.json. This may require explicit -m when developing and testing them. Thought I'd bring it up, don't know if there's a more convenient way to have the script react to this.

---

As it looks like a window for merging is coming soon, I'll compile some of the earlier points:

• Increased timings
• Unknown documentation error
• Bump Source version?
• body() or toSpawn() called first as of now? May impact CSG's tab should it see use in Uppsala
• Common Component edits
• See also comment in Improvements to the Module System #148 (comment)