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

Version 3 #470

Draft
wants to merge 182 commits into
base: master
Choose a base branch
from
Draft

Version 3 #470

wants to merge 182 commits into from

Conversation

ethanwu10
Copy link
Member

Major codebase refactor / client rewrite

Draft PR for CI runs

Closes #424

ethanwu10 and others added 15 commits August 8, 2020 02:39
Move configuration for TypeScript, configuration for NYC, and tests to
inside of the server/ directory.
Hoist shared TS ESLint config options to project root

Import from react / use react types to make typescript happy.

Add prefresh config: Webpack 5 support for prefresh has not yet landed,
but set up the configuration anyways.
Split lint script into separate server and client lint jobs, use `types`
option in tsconfig to restrict global type loading, explicitly define
`process.env` in client.
Roll back to Webpack 4 because Storybook does not support Webpack 5 for
the near future (see
storybookjs/storybook#11326 (comment)).

Use @storybook/react (which includes React as a dependency) instead of
Preact because the Storybook build breaks when using theme-ui (most
likely because of the bundled React dependency).
Factor out Babel config to babel.config.js so Jest can also use it. Also
set up globals handled by ProvidePlugin in Jest.

@types/history@2 added as a dependency to force yarn to not hoist
@types/history@^4.7 required by @types/reach__router, which is required
by storybook somewhere. This ensures TypeScript resolves 'history' as
@types/history@^4.7 instead of history@^5, which would otherwise happen
if yarn hoisted @types/history.
Jest is running with TypeScript Babel preset to transpile server code
and run against the TS sources; as a result, the leaderboard worker
needed to be mocked out, since worker_process bypasses the transpiler.
Always run codecov collect step if the tests ran (regardless of success
/ failure), and run client tests too.
@codecov
Copy link

codecov bot commented Aug 9, 2020

Codecov Report

Merging #470 (eedd1e8) into master (c0bc433) will decrease coverage by 8.90%.
The diff coverage is 30.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #470      +/-   ##
==========================================
- Coverage   60.26%   51.35%   -8.91%     
==========================================
  Files          87       89       +2     
  Lines        1384     1552     +168     
  Branches       57      300     +243     
==========================================
- Hits          834      797      -37     
- Misses        525      734     +209     
+ Partials       25       21       -4     
Impacted Files Coverage Δ
packages/client/src/api/mutations.ts 0.00% <0.00%> (ø)
packages/client/src/api/util.ts 0.00% <0.00%> (ø)
packages/client/src/app.tsx 0.00% <0.00%> (ø)
packages/client/src/components/navbar.tsx 0.00% <0.00%> (ø)
packages/client/src/config.ts 0.00% <0.00%> (ø)
packages/client/src/index.tsx 0.00% <0.00%> (ø)
packages/client/src/routes/home.tsx 0.00% <0.00%> (ø)
packages/client/src/util/auth.tsx 0.00% <0.00%> (ø)
packages/server/src/.eslintrc.js 0.00% <0.00%> (ø)
packages/server/src/api/admin/challs/delete.ts 0.00% <0.00%> (ø)
... and 166 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0bc433...eedd1e8. Read the comment docs.

Use project references for server (source and test), and also pull
shared config options up where possible.
Split config loading steps into individual exported functions; this
allows for fine-grained control over config in tests by mocking the
`load` module. Tests are set up to load config from its own conf.d, but
inherit critical environment settings like database connections from the
current (actual) config.
Use preact-render-to-string since preact-render-to-json does not
actually render components in Preact X. Also add stories for navbar
components.
Make client Jest config explicitly specifiy CWD for babel, so it can be
run from any directory. Set top-level `test` script to use `--projects`,
and also add a pre-commit hook to run Jest in only-changed mode.
Use mock factory to ensure config mocking works in all environments;
this fixes crashes with the full app startup integration test (and
therefore also fixes CI tests). Also add `--forceExit` to lint-staged
command.
ethanwu10 and others added 30 commits May 9, 2021 15:45
A few majors held back for another commit, Prettier re-run; theme-ui was
upgraded with a breaking deprecation that will be fixed in the next
commit.
`useCustomProperties` is now under `theme.config`.
Fix CI and Docker build to use yarn 2 commands, run `yarn dedupe` on
pre-commit and CI, and update a few more configs for yarn 2 paths and
conventions.
Disable diffs and auto merging on vendored yarn code, disable diffs on
yarn.lock, and remove .yarn/sdks from repo.
Merge in changes to Fastify request log serializer from upstream
Fastify. This fixes a deprecation warning.
Instead of using the object returned by `render`, query using the global
`screen` as recommended by Kent C. Dodds.
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#not-using-screen
Since the `getBy*` functions throw when they cannot find an element,
prefer them for querying for elements that should exist (except when the
test is explicitly for whether or not the element exists). This removes
most of the null checks in the test code.
Since yarn 2 uses an internal shell parser[1] which supports environment
variables, we no longer need to use cross-env for cross-platform
environment variable setting.

[1]: https://github.com/yarnpkg/berry/tree/%40yarnpkg/shell%2F2.4.1/packages/yarnpkg-shell
Fix .yarn being included twice, add dockerignore entries for yarn to
hopefully prevent some churn (using the ignore list recommended for
PnP zero-install).
When casting the handler functions exported from individual routes,
derive the target type by inspecting the `route` method of the
FastifyInstance we are registering them on. Since we adjust the instance
type to better match `http.Server`, this fixes a type mismatch that
appears when compiling on some platforms when trying to register the
handlers.
migrate to yarn 2 + batch dependency upgrade
Break ties between users with solves on challenges not marked
`tiebreakEligible: false` the same, but also rank users with only solves
on challenges marked `tiebreakEligible: false` via solve times on those
challenges. This does not change existing rankings, but adds users with
only solves on these challenges to the leaderboard as well.

Co-authored-by: Philip Papurt <[email protected]>
Remove intermediate calculation bookkeeping data from the types returned
out of the leaderboard worker. This change also strips the data from the
objects, resulting in the creation of a new JS object per leaderboard
entry, but keeps snapshots cleaner. We may wish to consider doing this
in only snapshot serialization in the future.
Small refactor of implementation of leaderboard cache `getRange` to
leverage type narrowing on the range parameters to eliminate a pair of
casts. Also restrict the `all` parameter to `true` only.
chore: cherry-pick changes from master
refactor(server): leverage narrowing on cache typing
chore: backport ranking users with only tiebreakEligible: false
`fastify-static`'s types have been fixed, making the previous cast
unnecessary now; remove it.
Previously `endTime` was set to 2021-06-10, which is now in the past and
causing tests regarding flag submission to fail since the CTF is now
closed. Year 3000 should be good enough right?
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.

The Great Client Rewrite
6 participants