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

100 items can be undefined #101

Merged
merged 2 commits into from
Dec 17, 2023
Merged

100 items can be undefined #101

merged 2 commits into from
Dec 17, 2023

Conversation

daelmaak
Copy link
Owner

  • fix: check that changes are defined
  • chore: use strict type checking in the gallery lib

@daelmaak
fix: check that changes are defined
01e51b6 
It can easily happen that items are undefined as a change when
ngOnChanges is called in GalleryComponent. This is a deficiency of the
Angular's types that this is not checked, and it slipped through my test
suite as well.

So I created a stricter type that will keep the possibility of changes
being undefined in check and reorganized the test so that the ngOnChanges
with undefined `items` change is checked as well.
@daelmaak
chore: use strict type checking in the gallery lib
3d4a7ae 
The loose type checking was one of the reasons the gallery was plagued
by TypeErrors lately. Well what do I have TS then for? Setting TS to
strict mode helps greatly with eliminating these.

Since lot of changed, let's go through the list of my decisions:
- I only made the lib's type checking strict since otherwise there would
  be too many changes. I don't want to enable this for the apps/demo
  since I kinda like having it flexible. Switching it on makes sense for
  the tests, but it's not too important at the moment, and I sense a
  small rewrite will be needed due to some of the less orthodox test
  code.
- I changed most of the inputs, especially in the child components, to
  optional so to reflect they might be undefined. But, few of those I
  declare as always defined. Though I don't like this, it's still better
  option than check all over the code base when I know the input is
  guaranteed to be defined from parent components.
- On many TemplateRefs I used `$any()` since Angular's type requirements
  are insane in certain contexts. It's especially maddening when it
  comes to Angular expecting `null` and me having `undefined`. In order
  not to have to do undefined | null gymnastics in my inputs, I chose
  $any as an easy way out.
- I usually declare ElementRefs as always defined and just accept the
  risk of theoretically running into TypeErrors. The reason is that I
  just feel that only because these are not existent only in the very
  early phases is not worth the trouble of always checking their
  presence. I am fully aware I prefer convenience over safety here, but
  I am convinced I can cover the blind spots with tests and good
  coding practices.
@daelmaak daelmaak linked an issue Dec 17, 2023 that may be closed by this pull request
items can be undefined #100
Closed
@daelmaak daelmaak merged commit d270ef9 into master Dec 17, 2023
1 check passed
@daelmaak daelmaak deleted the 100-items-can-be-undefined branch February 19, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

items can be undefined
1 participant
@daelmaak