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

fix: Enable strict TypeScript settings #39

Merged
merged 1 commit into from
Jul 24, 2020
Merged

fix: Enable strict TypeScript settings #39

merged 1 commit into from
Jul 24, 2020

Conversation

joachimvh
Copy link
Member

Sets TypeScript to strict

Biggest changes:

  • I made the webId field in Credentials optional. This way the CredentialsExtractor always returns a Credentials object, even if no credentials were found. (Instead of returning undefined in that case).
  • BodyParser now returns Representation | undefined. Not convinced that this is the best solution, but it is what is needed for now since the SimpleBodyParser also handles the case of there being no body. Perhaps this should be caught in the RequestParser instead. Related to Handle HttpRequests with no body #8 .
  • There are some cases where a check that was in the canHandle function was needed again in the handle function to prevent accessing potential undefined data since some input fields are optional. E.g. the method field in a HttpRequest. Another option would be to leave behind that duplicate check and tell TypeScript to trust us that the data is there (since it should have been checked in canHandle). Not that big of an issue but might become a problem in the future if more advanced checks are needed.
  • The ! operator was used in some cases to convince TypeScript that it shouldn't worry. Specifically in these cases:
    • In constructors where Object.assign was used to assign values to the class variables. The only way to get around that would be to assign them all manually which can get ugly if there are many. Related TypeScript issue with people complaining about this here (so far it has not been fixed).
    • One specific instance where an object was made right before the loop going over its keys to have cleaner code. Having a check there seems unnecessary.
    • Some tests since it just makes some things easier there.

@RubenVerborgh
Copy link
Member

Excellent!

* `BodyParser` now returns `Representation | undefined`. Not convinced that this is the best solution, but it is what is needed for now since the `SimpleBodyParser` also handles the case of there being no body.

I wonder if we want an EmptyRepresentation singleton.

Copy link
Contributor

@rubensworks rubensworks left a comment

Choose a reason for hiding this comment

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

Looks great :-)

@joachimvh joachimvh merged commit dcff424 into master Jul 24, 2020
@joachimvh joachimvh deleted the strict branch July 24, 2020 11:30
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.

None yet

3 participants