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

New Module Compilation System #163

Merged
merged 59 commits into from
Feb 12, 2023
Merged

Conversation

leeyi45
Copy link
Contributor

@leeyi45 leeyi45 commented Jan 8, 2023

Motivation

Currently module compilation times are in the order of several minutes. It would be great to reduce that time.

Replacing Rollup and Babel

esbuild is an extremely fast Javascript bundler which replaces the functionality provided by Rollup with Babel.

This also significantly reduces the number of dependencies that have to be installed. Rollup and Babel each required many packages and plugins. As far as I am aware esbuild is able to replace them, but I haven't been able to check for every single module and every single use case that the code functions exactly the same.

esbuild also provides built in minification, but that is currently left off as it may affect module code. It is already present should we wish to utilize this function in the future.

Watch Mode

Running yarn watch will run esbuild in watch mode, where tabs, bundles and documentation will be automatically rebuilt when changes in the file system are detected, You can disable building of documentation using the --no-docs option.

Linting and typechecking won't run by default, but you can use --tsc and --lint to run the Typescript compiler and eslint.

Replacing ts-node

Script code is written in Typescript, so currently ts-node is used to to run scripts. This is also a major bottleneck. Now, script code is compiled to Javascript and node is used directly.

RFC: Should the compiled script code be committed, or should it just be the source code, then every developer has their own compiled version?

Testing

I have run some tests on a fairly old Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz Windows machine with 16GB of RAM, and also on my M1 Pro MacBook Pro

yarn build --tsc --lint

Runs eslint, tsc (for type checking), esbuild and typedoc.

Windows MBP
Average Time (s) 50 14

yarn build modules

Same as yarn build, but documentation is not rebuilt

Windows MBP
Average Time (s) 45 12

yarn build modules

Only build bundles and tabs without linting, type checking and documentation.

Windows MBP
Average Time (s) 4 <1

Removal of database.json

With compilation times greatly reduced, the current system that checks against the last modified time of a bundle or tab is no longer required and has been removed to speed up the process.

Once a proper versioning system for modules has been introduced, this feature can be reintroduced.

Breaking Changes

ES5 to ES6

esbuild is not able to compile code down to ES5, which is the current compilation target in use. @martin-henz has indicated that there is no need to support ES5, so this PR will bump the lowest supported version of Javascript to ES6

type: 'module'

To further increase compatibility of the script system with ESM only packages, this field has been set in package.json. This has required .eslintrc.js files to be renamed .eslintrc.cjs. Configuration files that support ESM (i.e jest) have also been changed to ESM. This doesn't affect module code as far as I am aware.

Bug Fixes

  • Previously the json documentation would sometimes generate with an incomplete description of the function or variable. This has been fixed.
  • In line with NodeJS's intention to phase out --experimental-node-specifier, the scripts will use imports that conform to the ES standard instead, i.e. with file extensions and no directory imports.

Should fix the plethora of issues with the build system namely #164, #165 and #166

@leeyi45 leeyi45 changed the title Module Compilation Speed Improvements New Module Compilation System Jan 20, 2023
@leeyi45 leeyi45 marked this pull request as draft January 20, 2023 13:25
@leeyi45
Copy link
Contributor Author

leeyi45 commented Jan 20, 2023

esbuild has since been updated with some new features that I think this PR can take advantage of, give me some time to add them in

@leeyi45
Copy link
Contributor Author

leeyi45 commented Jan 30, 2023

With regards to csg, I'm finding a way to import React directly. The current method to pass React to the tabs isn't compatible with the new JSX transform either way.

esbuild's dev server rebuilds on each HTTP request, which resulted in the freezing observed by @Cloud7050. On the build system's end, I've made it such that esbuild only runs in watch mode. You can still use the included HTTP server package to serve modules. If that is too heavyweight yet you can just run yarn build which only builds on demand.
The second part to this issue is of course #79, where the frontend isn't yet capable of loading modules asynchronously.

With regards to typechecking and linting, I am considering running both tsc and eslint programmatically as well to see if any gains can be made in parallelizing that process.

@martin-henz
Copy link
Member

Would be great to have these improvements in place before the student teams get serious about PRs in the modules repo.

@martin-henz martin-henz self-requested a review February 12, 2023 01:12
Copy link
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

A consensus emerged that this PR is of strategic importance. @Cloud7050 reviewed and made comments. The CSG module was a showstopper, but the CSG team confirmed that CSG is now working with this PR.

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