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

Improvements to the Module System #148

Closed
wants to merge 52 commits into from
Closed

Conversation

leeyi45
Copy link
Contributor

@leeyi45 leeyi45 commented Aug 6, 2022

Refer to this PR and this PR for the related changes to the frontend and js-slang respectively.

Module Documentation

Currently, only the documentation for builtins is available through editor tooltips. This PR intends to make modules' documentation available as well.
image

This system piggy backs off how bundles and tabs are retrieved from the module server and makes it available to the frontend through js-slang.

Changes to the module build process

The new structure of the build folder is as follows:

build
├── documentation
├── jsons
│   ├── curve.json
│   ├── csg.json
│   └── // etc....
├── tabs
├── bundles
├── modules.json
└── database.json

Building Documentation/JSONS

The building of HTML documentation through Typedoc remains unchanged. To make module documentation available, the Typedoc JSON output is read to produce a json file for each module that will be located in build/jsons, allowing it to be served alongside tabs and bundles.

Docstrings written in the bundles are interpreted as markdown and converted to the HTML format that the frontend uses.
For example:

// curve/functions.ts
/**
 * This function is a curve: a function from a fraction t to a point. The points
 * lie on the unit circle. They start at Point (1,0) when t is 0. When t is
 * 0.25, they reach Point (0,1), when t is 0.5, they reach Point (-1, 0), etc.
 *
 * @param t fraction between 0 and 1
 * @returns Point on the circle at t
 */
export function unit_circle(t: number): Point {
  return make_point(Math.cos(2 * Math.PI * t), Math.sin(2 * Math.PI * t));
}

gets translated to the following JSON entry:

  "unit_circle": "<div><h4>unit_circle(t: number) → {Point}</h4><div class=\"description\"><p>This function is a curve: a function from a fraction t to a point. The points\nlie on the unit circle. They start at Point (1,0) when t is 0. When t is\n0.25, they reach Point (0,1), when t is 0.5, they reach Point (-1, 0), etc.</p></div></div>"

which is then displayed by the frontend as in the picture above.

Documentation can be built using the yarn build:docs command.

Building docs always builds the HTML documentation for all modules, as Typedoc overwrites the output folder every time it is run. Excluding bundles from its entry points would cause those modules' HTML documentation to be missing.

New Build Scripts

The command yarn build:modules will build both bundles and tabs. This is necessary because some tabs import from their parent bundles. The command yarn build:docs will build HTML and JSON documentations, as described above. yarn build will build both.

Build scripts will be able to take a variety of options:

  1. --force build all tabs, modules and JSONS regardless of build status
  2. -m or --modules. Specify modules to build. Usage: -m curve rune
  3. -tor --tabs. Specify tabs to build. Usage: -t Curve Rune
  4. -j or --jsons. Specify jsons to build. Usage: -j curve rune
  5. -v or --verbose. Display the reason why each item is being built.

The -t and -m options are ignored when running build:docs, and -j is ignored when running build:modules.

The build scripts have been designed to be written as ES modules, and so far have had no issues with ESM only packages, but unfortunately in my experience getting this to work, ESM only packages often don't play nicely.

yarn build:modules

If run without options, bundles or tabs will be rebuilt for the following reasons:

  • Missing output folder
  • Corresponding file is missing from output folder
  • Bundle or tab was modified after the timestamp indicated in database.json
  • Tabs will be rebuilt if their parent bundle was modified, regardless if the tab's source files were modified.
    If the --force option is specified, then all of these are ignored.

database.json

PR #141 stored the timestamp of the previous run of build. This PR adds on to that change by storing separate timestamps for each tab, bundle and JSON file, allowing build:modules and build:docs to only rebuild their respective outputs for tabs/bundles that have been modified since their last run, or if the respective build folder is empty or missing.

All commands can be run with the --force flag to force all outputs to be built, regardless of their modification status.

Further motivated by #79, database.json will now be available alongside modules.json, allowing js-slang, in the future, to be able to compare against the version of the modules/tabs/documentation it is currently using to the ones being served. Currently the database stores timestamps, but in the future it can probably be improved to use version numbers.

Changes to .eslintrc.js and tsconfig.json

To provide separate eslint configurations for build scripts and the modules' source code, .eslintrc.js and tsconfig.json are now located within the src folder. This PR has already modified the scripts within package.json to work with this change, but developers may want to take note that running commands like tsc may require the project flag.

A second tsconfig.json is located in the scripts folder for use by ts-node when running build scripts, along with a second .eslintrc.js for linting scripts.

vscode users may need to add the following to .vscode/settings.json to make the eslint extension work properly:

"eslint.workingDirectories": ["src", "scripts"]

Removal of prettier

I had trouble getting prettier to work with this multiple directory setup, just like eslint, but given that prettier has the same functionality as eslint, I think it is unnecessary and can be removed from the project. Should prettier's rules be desirable, it's eslint plugin can be installed and used.

Changes to Tab transpliation

Tabs are transpiled to the iife format by rollup, resulting in code that has the following structure:

(function (React, ReactDOM) {
  // Tab code...
}(React, ReactDOM))

Currently, js-slang is responsible for converting this raw tab format into the format needed by the frontend, which looks like this:

(function (React, ReactDOM) {
  // Tab code...
})

This means that every time that js-slang loads the tab, js-slang has to make the modification, which is simply unnecessary if the tabs are provided to it in the desired format to begin with.

With this PR, building tabs will now produce the latter format, removing the need for js-slang to process the raw tab file.

Updating Typedoc

The CSG module leverages a newer version of typescript, which required that Typedoc be updated. However, this has broken the original Source Academy Modules theme. Building HTML documentation will utilize typedoc's default theme currently.

@leeyi45
Copy link
Contributor Author

leeyi45 commented Aug 6, 2022

Would like some feedback on the Gulp integration and the changes made to .eslintrc.js and tsconfig.json

@Cloud7050
Copy link
Contributor

If the system for interactively setting up modules has been removed, then it would be safe to remove its script and script info from package.json

@Cloud7050
Copy link
Contributor

Cloud7050 commented Aug 10, 2022

Would like some feedback on the Gulp integration and the changes made to .eslintrc.js and tsconfig.json

For ESLint, it may be preferable to keep the rules used between the various files in the repo similar. #133's changes to the config are such that the base file configures all JS rules (thus airbnb is no longer extended for JS), and the main config overrides for TS files to build atop the JS rules. In this PR, further overrides for a subset of JS/TS files, that also extend (with airbnb) in a way that overwrites many of the existing configured and more broadly applied JS rules, appear to result in the rules used to lint those specific files being rather different. A few rules are then manually turned off again since airbnb turned them back on. Could probably get away with only overriding to add plugins, while extending to add on the rules for those plugins without overwriting existing rules.

Side note, the latest version of the airbnb config used here has received a fair amount of changes compared to the version used prior to #133. Its new changes include no longer using some rules that this repo had disabled around the place with eslint-disable comments, comments which were no longer needed in the files and removed in #146.

The tsconfig move seems to make sense, but I just noticed that the trailing comma for the json looks a bit sus. Other than that, it may be good to standardise the relative paths like before: .eslintrc.base.js → back to ./.eslintrc.base.js, ./src/tsconfig.json unchanged (similar to the previous ./tsconfig.json), ../.

@Cloud7050
Copy link
Contributor

If a module's bundle file(s) contain changes (file gets saved with a newer timestamp), but that same module's tab(s) do not, or vice versa, do all of that module's files (both bundle and tabs) get rebuilt under this new system?

Treating a module's bundle and tabs as one unit is the current behaviour of the rollup quick rebuild system in master. This is because a module may import its own relevant content from files stored under a separate folder, which is the case with CSG's shared constants in the bundles folder. Ie changes to a file in the bundles folder will situationally impact the tab in the tabs folder, meaning the module as a whole needs rebuilding, not just the lone bundle whose contents trigger the isFolderModified check. It is thus safer to always rebuild whole modules, to avoid the developer having to remember to sometimes force a complete rebuild of all modules, lest they experience undefined behaviour.

@Cloud7050
Copy link
Contributor

Regarding the latest commit, LowDB V1 is used because LowDB V2 and up are pure ESM packages as mentioned in its docs, which don't play well with the rollup build process. This is a similar reason to why we are not upgrading the major version of Chalk, when I tried to do so a while back. I'm not familiar with the specifics, but if something has now changed, then that's great and we could upgrade Chalk too.

The new versions of LowDB also have an entirely different API. I initially wrote the new rollup system using LowDB V3, but had to downgrade everything and rewrite the relevant calls for V1.

@leeyi45
Copy link
Contributor Author

leeyi45 commented Aug 17, 2022

Unfortunately, to make typedoc work with the updated master branch, its version has had to be bumped, which has caused the original HTML theme to no longer work. Refer to this for more information.

I don't really know how to rebuild the original theme system to fit the new Typedoc system

@leeyi45
Copy link
Contributor Author

leeyi45 commented Aug 18, 2022

Also if someone knows how to make the eslint configuration work that would be great

@leeyi45 leeyi45 marked this pull request as ready for review August 22, 2022 08:33
@leeyi45
Copy link
Contributor Author

leeyi45 commented Aug 22, 2022

Also if someone knows how to make the eslint configuration work that would be great

I've since figured out a way to make both vscode and eslint itself happy, but feel free to make comments on the organization of the eslint configuration

@leeyi45
Copy link
Contributor Author

leeyi45 commented Aug 23, 2022

I noticed that the frontend and modules repo had different versions of @blueprintjs, resulting in weird behaviour when tabs were rendered. Possibly need to look into how we can update blueprint in the modules repo if it is updated in the frontend

Made it so that jsons don't have to be built every time HTML documentation is built
@Cloud7050
Copy link
Contributor

The CSG module leverages a newer version of typescript, which required that Typedoc be updated. However, this has broken the original Source Academy Modules theme. Building HTML documentation will utilize typedoc's default theme currently.

I was not aware of this. Could you clarify what you are referring to by typescript? Could it be some of the language or documentation features we are using in our code? It seems the version of the typescript package has not been changed for about 18 months. I tried to avoid updating packages unless necessary to avoid breaking things.

I am also not sure what this theming refers to. It would be great if you could elaborate. Is it the purple of the documentation pages? The last time I checked, CSG and the other modules were able to build this style of documentation while the modules were also built. As I understand it, this is not related to the new documentation in the suggestions of the Ace editor, and as the theme is broken, a default theme is used instead?

@martin-henz
Copy link
Member

This PR is subsumed by #153. Let's close it when #153 is merged.

@martin-henz martin-henz marked this pull request as draft September 26, 2022 09:30
@martin-henz
Copy link
Member

Closing because it's superseded by other PRs, as confirmed by @leeyi45 via telegram.

@leeyi45 leeyi45 deleted the editor_documentation branch November 3, 2023 14:51
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