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.

Sign up for GitHub

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

Jump to bottom

Adopt TypeScript #398

Open
thewilkybarkid opened this issue Aug 19, 2021 · 1 comment
Open

Adopt TypeScript #398

thewilkybarkid opened this issue Aug 19, 2021 · 1 comment

Comments

@thewilkybarkid
Copy link
Member

Currently, TypeScript is partially used on the backend, without type checking (@parcel/validator-typescript is a dependency, but isn't enabled). All files should be TypeScript (including the frontend etc), with strict mode enabled and types actually checked.
thewilkybarkid added a commit that referenced this issue Sep 7, 2021
@thewilkybarkid
fix(deps): remove forked dependency
318fdf7 
Remove use of a forked dependency. The fork appears to add in an 'optional' option, which is undocumented and seemingly untested. It seems to work without it too.

The dependency has seen some updates, too, including TypeScript changes required for using strict mode (CyriacBr/class-fixtures-factory#10).

Refs #406, #398
thewilkybarkid added a commit that referenced this issue Sep 7, 2021
@thewilkybarkid
fix(deps): add missing types
b9aa8b0 
Refs #398
thewilkybarkid added a commit that referenced this issue Sep 7, 2021
@thewilkybarkid
fix(deps): add missing types
81c51c8 
Refs #398
thewilkybarkid added a commit that referenced this issue Sep 7, 2021
@thewilkybarkid
refactor: remove unused function
a637a6d 
It's unused and appears to do nothing anyhow.

Refs #397, #398
thewilkybarkid added a commit that referenced this issue Sep 7, 2021
@thewilkybarkid
fix: improve types
d542241 
This change resolves some incorrect types, which don't trigger problems unless TypeScript is in a stricter mode.

Refs #398
@thewilkybarkid
Copy link
Member Author

One particularly troublesome line is

prereview/src/backend/utils/getFields.ts

Line 9 in d542241

return Object.values(entities[type].prototype.__meta.properties).reduce(

It seems to be relying on the ORM having added a __meta property to the prototype, which is all completely untyped (and so clearly a hack). It seems possible to find it for an entity (using wrap()), but not for the prototype. It's possible you could find it out through the ORM's metadata storage (not tried yet), but I haven't really got a clue what this code is trying to achieve anyway. It feels like there's a far bigger problem to solve.

thewilkybarkid added a commit that referenced this issue Sep 8, 2021
@thewilkybarkid
fix: use entity types
d1243cd 
Use of the 'any' type is dangerous and can easily result in bugs.

Refs #398
thewilkybarkid added a commit that referenced this issue Sep 8, 2021
@thewilkybarkid
fix: add type
391c2da 
The use of 'unknown' is better than 'any' because it's at least type-safe, but this should set the correct type.

Note this doesn't fix all the typing problems in this code.

Refs #398
thewilkybarkid added a commit that referenced this issue Sep 10, 2021
@thewilkybarkid
fix: add required locale setting
e7aaa48 
The IntlProvider has a required 'locale' setting; this didn't cause an error but make the console noisy while developing.

Refs #398
thewilkybarkid added a commit that referenced this issue Sep 10, 2021
@thewilkybarkid
refactor: use TypeScript for the entry point
0ea5b08 
Refs #398
thewilkybarkid added a commit that referenced this issue Sep 16, 2021
@thewilkybarkid
perf(images): don't load avatars when they're not going to be displayed
440b3e3 
Persona avatars are loaded, but the component expects them to be a data URI string rather than a Buffer object. As a result, the browser tries to load `/[object%20Object]`.

Images inlined in the API are one of the leading performance problems. Rather than try and fix the types so that avatars show consistently, this hides them completely where they don't work. As far as I'm aware, there's been no complaints that they're not displayed, and this will help make some of the critical pages a bit less slow.

Refs #392, #398
thewilkybarkid added a commit that referenced this issue Oct 12, 2021
@thewilkybarkid
fix: don't redefine an environment variable
85407a2 
__dirname is a Node.js environment variable that contains the path to the module's directory. It's currently redefined to be the current working directory, which is somewhat unexpected. This change removes it as path.resolve() uses the current working directory anyway.

Refs #398
thewilkybarkid added a commit that referenced this issue Oct 12, 2021
@thewilkybarkid
build: use TypeScript for Jest tests
883f01b 
While there aren't any Jest tests yet, this change sets up ts-jest so we can write tests in TypeScript.

Refs #388, #398
thewilkybarkid added a commit that referenced this issue Oct 22, 2021
@thewilkybarkid
fix: don't try and update the Fathom site ID if the frontend isn't there
4878faa 
If the frontend isn't available (probably not possible yet, but will be when testing the backend directly), there will be no response body. This change makes sure it's stream-like before trying to do so.

Refs #388, #398
thewilkybarkid added a commit that referenced this issue Oct 22, 2021
@thewilkybarkid
build: treat TypeScript modules as standalone
724a2bb 
As Parcel processes each file individually, TypeScript needs to be aware to disable incompatible types. Also, ts-jest needs to have type-checking disabled (since they don't currently pass).

Refs #398, https://parceljs.org/languages/typescript/#isolatedmodules
thewilkybarkid added a commit that referenced this issue Oct 27, 2021
@thewilkybarkid
refactor: use TypeScript for the logger
1152bd4 
Refs #398
thewilkybarkid added a commit that referenced this issue Oct 27, 2021
@thewilkybarkid
refactor: use TypeScript for the server
ade9e85 
Refs #398
thewilkybarkid added a commit that referenced this issue Nov 1, 2021
@thewilkybarkid
refactor: use TypeScript for generateDOI
08b08b9 
Refs #398, 400
thewilkybarkid added a commit that referenced this issue Nov 2, 2021
@thewilkybarkid
refactor: use TypeScript for Jest configuration
90585a0 
Refs #395, #398
thewilkybarkid added a commit that referenced this issue Nov 2, 2021
@thewilkybarkid
build(eslint): lint all file types
ba132cc 
ESLint appears on only lint .js files by default.

I expect this CI build to fail as it reveals linting problems in TypeScript files.

Refs #398, #422
thewilkybarkid added a commit that referenced this issue Nov 2, 2021
@thewilkybarkid
fix(eslint): recognise type usage
fccd24d 
I'm not sure why @typescript-eslint/typescript-estree has been used: I can't see any useful information about it. All documentation points to using @typescript-eslint/parser, which has been installed already. The only noticeable difference is that it doesn't understand when a type has been used, leading to @typescript-eslint/no-unused-vars errors.

I've added a separate TypeScript config file that includes all files in the codebase, as the new parser will error if it tries to load a file that hasn't been included.

Refs #398, #422, ba132cc
thewilkybarkid added a commit that referenced this issue Nov 2, 2021
@thewilkybarkid
fix: add types
3a0e1a9 
Refs #398, #422, ba132cc
thewilkybarkid added a commit that referenced this issue Nov 4, 2021
@thewilkybarkid
build: strictly typecheck the codebase
163bd96 
This change enables TypeScript's strict mode, which allows various bugs to be revealed and shows areas that remain untyped.

As there are many things to fix, I've added an optional step to the CI workflow.

Refs #398, #422
thewilkybarkid added a commit that referenced this issue Nov 4, 2021
@thewilkybarkid
build: don't typecheck when using ts-node
a8e982f 
As typechecking is separate from running code, ts-node needs to have it explicitly disabled. We've only noticed this problem once TypeScript's strict mode was enabled, as there are currently type problems.

As an aside, this will also be fractionally quicker.

Refs #398, #422, 163bd96
thewilkybarkid added a commit that referenced this issue Nov 4, 2021
@thewilkybarkid
fix(deps): add missing types
abc3e4f 
Refs #398, #422
thewilkybarkid added a commit that referenced this issue Nov 8, 2021
@thewilkybarkid
refactor: remove indirection
fdfd4b8 
This change removes the untyped getActivePersona function, which doesn't do anything useful.

Refs #398, #422
thewilkybarkid added a commit that referenced this issue Nov 8, 2021
@thewilkybarkid
refactor: use TypeScript for some middleware
72f9f95 
Refs #398
@thewilkybarkid thewilkybarkid mentioned this issue Nov 24, 2021
Closed
thewilkybarkid added a commit that referenced this issue Nov 24, 2021
@thewilkybarkid
fix: use the public API for reading entity metadata
e1ba4e6 
Rather than misusing a private decorator-specific API, this change uses the ORM's metadata storage instead.

A slight issue here is that accessing the entity manager from a repository is technically not a public API, but the code doing so isn't (yet) TypeScript and needs a lot of work anyway.

Refs #398, #399
thewilkybarkid added a commit that referenced this issue Dec 16, 2021
@thewilkybarkid
refactor: use TypeScript for all the scripts
d86fcc5 
Refs #398
thewilkybarkid added a commit that referenced this issue Dec 17, 2021
@thewilkybarkid
refactor: explicitly type fake objects
77eea55 
The OpenAPI generator script stubs dependencies that aren't run; this change avoids future typing problems by using an 'any' type.

Using 'any' is usually not advised, but it's more pragmatic given the alternatives.

Refs #398, d86fcc5
thewilkybarkid added a commit that referenced this issue Dec 20, 2021
@thewilkybarkid
refactor: use TypeScript for all the middleware
35937d9 
Refs #398
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@thewilkybarkid