## Summary
This PR significantly improves maintainability by breaking up the 880+ line monolithic app.ts file into smaller, focused modules with proper testing. The code is now organized into dedicated modules under the lib/app/ directory, making the codebase more maintainable and testable.
## Key changes
- Extract functionality into modules under lib/app/ directory:
- Command-line handling (cli.ts)
- Configuration loading (config.ts)
- Web server setup and middleware (server.ts)
- Core application initialization (main.ts)
- URL handlers, routing, rendering, and controllers
- Add comprehensive unit tests for all new modules
- Make compilationQueue non-optional in the compilation environment
- Improve separation of concerns with dedicated interfaces
- Ensure backward compatibility with existing functionality
- Maintain cross-platform compatibility (Windows/Linux)
## Benefits
- Improved code organization and modularity
- Enhanced testability with proper unit tests
- Better separation of concerns
- Reduced complexity in individual files
- Easier maintenance and future development
This refactoring is a significant step toward a more maintainable codebase while preserving all existing functionality.
The global handler_config was being used to allow compilers to find
other compilers. This change replaces that global with a proper method
on the CompilationEnvironment class, allowing compilers to find other
compilers through the environment instead of using a global variable.
🤖 PR description generated with [Claude Code](https://claude.ai/code) -
code by @mattgodbolt though :)
Co-authored-by: Claude <noreply@anthropic.com>
This PR removes the top-level global variable for the base directory and replaces it with a module-level variable managed through a setter function. Key changes include removing the global declaration in lib/global.ts, introducing a local ce_base_directory variable and setBaseDirectory function in lib/assert.ts, and updating app.ts to use the new setter.
---------
Co-authored-by: Claude <noreply@anthropic.com>
New node sentry prefers some node.js "preload" nonsense. I use this
"hack" to avoid having to make a lock-step change to the runner: tested
locally and then deployed to staging and tested there with both
server-side and client-side errors.
Main changes:
- type checker cares about the return value (`void`) of handlers, so no
more `return res.send("...")` as that returns `express` type.
- regexes on slugs no longer supported, but we weren't really using them
in any meaningful way. The two places that had to be updated:
- `/clientstate/:clientstate64` - now uses a regex directly and tests added (thanks @partouf for spotting #4844)
- `/bits/:bits.html` - was some `\w+` but I believe that's unnecessary
for the same reasons
- actually call the Sentry handler. I don't know if this actually worked
before but the API checks suggest not.
Remove most, if not all, of the synchronous file reads. Hopefully this
will help a little with performance and "event loop lag". Mostly, it's
"try not to use third party packages when builtins now do the work".
Local testing seems OK - but needs a good poke around on staging to
exercise all the paths.
Nothing was using it, we're using other methods these days to discover
remote compilers (and have been for years), and it was broken (see
#1790).
Also cleans up some config (moving it from aws to ceProps; though it was
unused, default used only), and also some workarounds for remote
compilers that refer back to the MS compiler era (#1768).
This leaves the ability in `aws.ts` to fetch instances in case we ever
need the functionality, as this is tested and still works.
Closes#1790
This PR addresses issue #7158 by removing the body-parser dependency in
favor of express.json() and other built-in middleware functionalities
provided by Express.js.
This has been dangling as a TODO from @mattgodbolt since 2021. Running
the following Athena query:
```sql
SELECT COUNT(*) as reqs, uri, date
from cloudfront_logs
WHERE uri LIKE '/shortener%'
GROUP BY date, uri
```
Yields about 8-20 requests per day. I think this is an insignificant
enough amount of traffic to justify removing it now.
This patch decouples the formatting logic (which is used by the
clang-format feature) from the http API logic.
This is done with the new FormattingService populates the formatters
from the config, and wraps formatter execution. This way, both the http
controller and the clang-format functionality can use it, reducing tight
coupling.
This is the first PR in API consistency with more to come:
1. The source API urls have been properly defined with
`/source/:source/list` and `/source/:source/load/:language/:filename`
- These used to be two API endpoints mushed together into one `handle()`
function. They are now separate
- While it may appear from the tests that this is an API breakage,
there's currently no way to have the source API spit out responses for
stuff like `/source/moose/load/Grunkle`.
2. The frontend code calling the list api now has shared types
3. Code has been migrated to `/lib/handlers/api/source.ts`
- I've moved code here, because I would like to separate the stuff
that's under the API and the stuff that isn't (e.g., healthcheck)
- The source endpoint is currently not under /api, but I believe it
makes sense to move it.
4. GitHub is terrible at showing the diff, but the `browser.ts` file has
been removed, since it's never actually used. Our frontend only calls
`/source/builtin/...` and it never had any entries. Besides, the type
used on the frontend for the locally stored stuff is different...
1. Gives a proper type to the metadata
2. Fixes a typo in `screenshot_dimentions`
3. Removes home-made partition function in favor of underscore
4. Reads file asynchronously (and consequently rest of API is async)
5. Strips the `https://godbolt.org/#` from each entry in the config file
As an added benefit this should also cut some startup time reading this
file before it's used :)
This includes among others:
- Proper tsification of various tools code,
- Elimination of the `CompilationInfo2` type,
- Use of `CompilationInfo` instead of `Record<any, any>` in some places
- These lines in CompilationResult:
```
// Temp hack until we get all code to agree on type of asm
asm?: ResultLine[] | string;
```
The next task would be to get all code to agree on the type of
CompilationResult.asm, thereby enabling fixing of most the remaining
TSification.
_No functional change intended_
According to
7ce90a3388
and
0d0a52e249
the route was added a very long time ago (4-5 years) to support compat
for workers downloaded to the user's browsers.
In addition to this being very long ago to the point that the chance of
anybody having an old CE worker being extremely low, those files no
longer exist.
We tag our webpack bundles with a "magic version" (current .v53.) to
force the browsers to re-fetch the bundles for major changes. That means
that if somebody had `editor.v10.worker.[hash].js` from years ago, that
file won't even be in our current bundle, and has to be in their
browser's cache (unlikely because it's so old, but even then their
browser wouldn't hit this route due to cache)
Instead of having several globals, set via environment variables,
explicitly set the "correct" env var if passed `--tmpDir` and then
consistently use it in the rest of the program.
See @apmorton's comments in #1707
Mindless replacements of the form
`_.filter(options, option =>...` --> `options.filter(option =>...`.
One not *entirely* mindless replacement at the bottom of
compiler-dropin-tool.ts :
```
- return _.filter(pathFilteredFlags) as string[];
+ return pathFilteredFlags.filter(Boolean) as string[];
```
6 files can now stop importing underscore.
Adds D8Compiler, which applies to the Android Java and Android Kotlin
languages. D8Compiler instantiates a JavaCompiler or KotlinCompiler
using the java/kotlin dependencies' paths for D8 in the infra repo.
compiler-finder.ts has been updated to allow for duplicate compiler IDs
for 'android-java' and 'android-kotlin', as it is expected that the
compilers used for these languages is the same.
Attempt to fix#5476. As far as I can tell `process.env.winTmp` is
needed just for windows executables and WslVcCompiler. It should be
possible to just ignore the exec error and continue. If for some reason
the user doesn't have windows paths in their WSL but does want to run
windows executables they can pass `-tmpDir`.
This PR refactors some common utilities out of lib/ and into shared/ and
eliminates some use of underscore.js, as well as general type
improvements done along the way.
