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

feat: new blog post about the injector #84

Merged
merged 15 commits into from
Jun 15, 2021
Merged

Conversation

georgiee
Copy link
Member

@georgiee georgiee commented Jun 8, 2021

During the day I took constant small breaks to write this small blog article.
I would like to see how it feels and what we are missing right now.

Our codebase is a little mess and I try to refactor code from the beginning bit by bit with every blog post. Naturally this firs blog post caused some more changes I wanted to have fixed.

  • The blog image was a cloudinary url directly in frontmatter. I would like to use a local image. This must be possible
  • I wanted to highlight lines in our code blocks. Did not find an easy way although the documentation tells it should work, but without an example.
  • I tried out to use the diff- highlighter. Works okay'ish, maybe the theme is not ideal supporting? Because colors are weirdly mixed.
  • A header image is missing. Or any image at all 😅 That makes it a really rough reading experience and I would like to add something soon.
  • There might be some typos, I still have to read it a second time.
  • Updated the hero image handling. I think the one integrated in the Layout is now obsolete but I didn't dare to remove it just yet as I don't know about the implications for content outside of the blog.
  • Unlocked the blog in the navigation
  • Added reading time

@georgiee
Copy link
Member Author

georgiee commented Jun 8, 2021

@feedm3
FYI, I had to reactivate the gatsby runner manually because it was suspended due to inactivity. Fair enough for the free tier though.

Screenshot 2021-06-08 at 15 32 37

Screenshot 2021-06-08 at 15 32 57

@georgiee georgiee force-pushed the gk-blog-post-angular-injector branch from 55b49ad to 13e59af Compare June 8, 2021 13:51
@georgiee georgiee force-pushed the gk-blog-post-angular-injector branch from 13e59af to 642ef1a Compare June 8, 2021 16:01
@gatsby-cloud
Copy link

gatsby-cloud bot commented Jun 8, 2021

Gatsby Cloud Build Report

satellytes.com-main

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 1m

Performance

Lighthouse report

Metric Score
Performance 🔶 78
Accessibility 💚 95
Best Practices 💚 100
SEO 💚 90

🔗 View full report

@georgiee georgiee force-pushed the gk-blog-post-angular-injector branch from 2611eae to ea72474 Compare June 10, 2021 06:06
@georgiee georgiee requested a review from feedm3 June 10, 2021 08:05
@georgiee
Copy link
Member Author

The preview looks quite good (via https://metatags.io/) but the excerpt is missing. I will add the change.
Screenshot 2021-06-10 at 10 06 28

src/components/util/use-media-query.ts Outdated Show resolved Hide resolved
@georgiee
Copy link
Member Author

That looks great now too ☀️

Screenshot 2021-06-10 at 10 32 40

@feedm3
Copy link
Member

feedm3 commented Jun 10, 2021

It really cool, I like it. Looking forward to more blog posts 💪

What I don't like is the code formatting. It's just my taste but the Github code formatting looks way better. I created a small example gist. Maybe we can use this formatting as inspiration. Doesn't matter for merging this PR though, more of a future improvement.

https://gist.github.com/feedm3/c366ac7f00e3cbd5de899e78bae404e4

edit: github includes the gist right into the preview, nice

@georgiee
Copy link
Member Author

georgiee commented Jun 10, 2021

I fully agree with the syntax highlighting. I added the default syntax from prism. Much better than the "coy" version I used for the light design (see https://prismjs.com/) but the diff highlight is still off. Let's go with this and have a dedicated session for the syntaxt highlight then. There must be a github flavoured prism style out there already.

Screenshot 2021-06-10 at 10 50 20

Copy link
Contributor

@SyFlo SyFlo left a comment

Choose a reason for hiding this comment

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

a very pleasant and interesting read 😃 I made some suggestions, mostly about wording and correcting the odd typo.

blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
That base class provides a set of default functionality for any other component extending from it in the future. This not only saves repeated work on the side of the component authors. This acts as an alignment & contract between all derived components.

The `MyAbstractBaseComponent` will be delivered through a core library and extended by dozens of other components.
Like the following imaginary subscription component:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to merge this into one sentence ."..by dozens of other components like the following imaginary subscription component:"
It's not that I don't like the imaginary component that follows ;-) just that the word "like" should rather be interpreted as "for example" in this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I like it and it's prepare on my local changes 😄


The `SubscriptionComponent` is guaranteed to invoke the one service through `ngOnInit` as described by the parent class plus it can access the `subscribe` method from the parent class as the service `mySubscribeService` is readily available to be invoked by `subscribe()`.

Things get complicated the moment library authors starts to get a little more advanced.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Things get complicated the moment library authors starts to get a little more advanced.
Things get complicated the moment library authors start to get a little more advanced.

Just a little typo. However, the first sentence tripped me up a bit. Nothing to put my finger on, but the choice of words "authors getting advanced" seems a bit odd and I'm not sure everyone will immediately understand that you're talking about the programming experience of authors. Just a suggestion, maybe something along the lines of "Things get complicated the moment library authors implement slightly more advanced use cases"?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the local changes batched like this. Thanks.

Things get complicated the moment library authors implement slightly more advanced use cases, like in the following component.

blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
## Conclusion
Handling breaking changes is an act of empathy 💛. You want to protect your users from struggling with your changes. The "inject the injector" pattern we have introduced here helped us a lot and made it very easy to extend our base class without breaking things anymore.

It's always advisable to be more specific instead of using generalized concepts like described in this blog post. Your code will be less abstract, better readable and maintainable. Use this approach only if you have very good reasons to do so 👍 This is a highly specific pattern for our distributed library project. In case you're developing individual components that are not distributed to other developers you most probably should not care for breaking changes and patterns like that.
Copy link
Contributor

Choose a reason for hiding this comment

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

"It's always advisable to be more specific instead of using generalized concepts like described in this blog post." - I'm not sure I understand what you mean to say by that. Is it synonymous to
"It's always advisable to be as specific as possible rather than using generalized concepts like the one we describe in this blog post"?
The last sentence in this paragraph suggests that this is supposed to be disclaimer of sorts. I think I get what you're saying, but maybe something along the lines of
"In case you're developing individual components that are not distributed to other developers you most probably have no need to prevent the occurrence of breaking changes by means of patterns like that"
would be a better choice of words? What developer in their right mind would not care for all sorts of patterns and breaking changes? I think what we want to say is that unless you find yourself working with a large codebase and distributed teams in a typical enterprise organization, dealing with breaking changes is much easier and introducing a pattern like this might not be necessary or even beneficial.

Copy link
Member Author

Choose a reason for hiding this comment

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

You understood me correctly. Actually I wrote this as a word of warning at the beginning of the blog post thinking of the people taking such a pattern and over complicating their simple project. I wanted to clarify that this is only suitable for distributed libraries. I like your last summary and here my updated sentence:

Introducing a pattern like this might not be necessary or even beneficial if you are not working with a large codebase and distributed teams in a typical enterprise organization. Never use this pattern only to save a few lines of code, you will make your code less understandable.

src/components/cards/blog-card.tsx Outdated Show resolved Hide resolved

const BlogHeroImage = ({ wideImage, squareImage }) => {
/**
* A screen is tall once the height is larger then the width, which means the aspect ratio dips below 1 (0-1)
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment says "A screen is [considered as] tall..." but I don't see any mentioning of tallness in the code below. As a casual reader this puzzles me a bit, maybe the comment could rather explain that the output of either one of HeroImageDefault or HeroImageSquared is visually hidden depending on the viewport aspect ratio. This gets apparent by looking at the styled components directly above, but giving a reference to "aspect-ratio" rather than "tallness" might save the reader a second or two.

src/templates/blog-post.tsx Outdated Show resolved Hide resolved
date: "2021-06-08"
title: "The \"Inject the Injector\" pattern"
featuredImage: images/georgios-kaleadis-aBTfTMsOCOI-unsplash.jpg
imageCredits: We want to credit people for their work
Copy link
Contributor

Choose a reason for hiding this comment

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

should this actually list people to credit for images?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, then I switched plans and just took my own image but actually I would like to ship this feature with this PR so I try to push a change along the text update. Thank for the hint!

Co-authored-by: SyFlo <68643437+SyFlo@users.noreply.github.com>
Copy link
Member Author

@georgiee georgiee left a comment

Choose a reason for hiding this comment

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

comments for the pending review, but I think they are too late because I just accepted the batch of changes I commented on 😄

date: "2021-06-08"
title: "The \"Inject the Injector\" pattern"
featuredImage: images/georgios-kaleadis-aBTfTMsOCOI-unsplash.jpg
imageCredits: We want to credit people for their work
Copy link
Member Author

Choose a reason for hiding this comment

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

yes, then I switched plans and just took my own image but actually I would like to ship this feature with this PR so I try to push a change along the text update. Thank for the hint!

blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
That base class provides a set of default functionality for any other component extending from it in the future. This not only saves repeated work on the side of the component authors. This acts as an alignment & contract between all derived components.

The `MyAbstractBaseComponent` will be delivered through a core library and extended by dozens of other components.
Like the following imaginary subscription component:
Copy link
Member Author

Choose a reason for hiding this comment

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

I like it and it's prepare on my local changes 😄


Can you see the elegance here? We inject the injector, which is the engine of the dependency injection (DI) system in Angular itself and then request the singleton instances of our desired services to assign them to the local variables as before.

The injector has the correct typing, that the result might be undefined, that's why we convince TypeScript with the [non-null assertion operator (!)](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html#non-null-assertion-operator) that we guarantee to receive a value. That's possible because we control the environment, and the services are guaranteed to be available as they are provided in the root (`@Injectable({providedIn: 'root' })`).
Copy link
Member Author

Choose a reason for hiding this comment

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

yes indeed. I think the sentence is a left over how I developed the sentence in my mind where I'm coming from the problem and then justify it by the operator. Sounds much better the other way around.

blog-posts/angular-inject-the-injector.md Outdated Show resolved Hide resolved
}
```

resulting in this much more compact version of a constructor.
Copy link
Member Author

Choose a reason for hiding this comment

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

I added the following changes locally. Thank you

The result is a much more compact version of the original constructor.

## Conclusion
Handling breaking changes is an act of empathy 💛. You want to protect your users from struggling with your changes. The "inject the injector" pattern we have introduced here helped us a lot and made it very easy to extend our base class without breaking things anymore.

It's always advisable to be more specific instead of using generalized concepts like described in this blog post. Your code will be less abstract, better readable and maintainable. Use this approach only if you have very good reasons to do so 👍 This is a highly specific pattern for our distributed library project. In case you're developing individual components that are not distributed to other developers you most probably should not care for breaking changes and patterns like that.
Copy link
Member Author

Choose a reason for hiding this comment

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

You understood me correctly. Actually I wrote this as a word of warning at the beginning of the blog post thinking of the people taking such a pattern and over complicating their simple project. I wanted to clarify that this is only suitable for distributed libraries. I like your last summary and here my updated sentence:

Introducing a pattern like this might not be necessary or even beneficial if you are not working with a large codebase and distributed teams in a typical enterprise organization. Never use this pattern only to save a few lines of code, you will make your code less understandable.

src/components/cards/blog-card.tsx Outdated Show resolved Hide resolved
@georgiee
Copy link
Member Author

I added attribution data to the markdown so we can use images with the required attribution, which is just fair to the creator 👍

attribution:
  creator: upklyak
  source: https://www.freepik.com/free-vector/space-landscape-night_13748451.htm

Screenshot 2021-06-15 at 10 39 15
Screenshot 2021-06-15 at 10 39 19

Links are nofollow and blank too. Let's finally merge this blog post to write the next one 👍

@georgiee georgiee merged commit cb6e2c8 into main Jun 15, 2021
@georgiee georgiee deleted the gk-blog-post-angular-injector branch June 15, 2021 08:46
@georgiee
Copy link
Member Author

Thanks for your reviews, very much appreciated 💛

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.

3 participants