fix: add missing flarum/tags as optional dependency

[DavideIadeluca]

The extension will fail to initialize in some cases

[jaspervriends]

Hi! Thanks for the PR. The idea of the current setup is that no other extensions are required to be installed. Are you trying to fix #98 or are you experiencing a different issue with the tags extension?

[DavideIadeluca]

Hey @jaspervriends thanks for getting back to me. Yes I was first and foremost talking about the issue reported in #98. Now it seems like the line causing the issue there wasn't needed in the first place and got removed, so that in itself is resolved.

There are other parts in the codebase though that build upon tags, so I think the way to go here is add flarum/tags as an optional dependency to make sure the order of initialization is correct.

I would also suggest making use of the Conditional Extender and cleaning up extend.php a little. Would you be okay with this approach? Will as a result require flarum/core v1.8.3 or newer. Happy to propose that in a PR

[jaspervriends]

Now it seems like the line causing the issue there wasn't needed in the first place and got removed, so that in itself is resolved.

The line indeed wasn't actively in use, but the idea behind it was to be able (see https://community.v17.dev/knowledgebase/46 Query meta dialog by model hidden in the spoilder below 'Querying by object Type and ID' and makes this approach work: https://github.com/v17development/flarum-seo/blob/master/js/src/common/Components/MetaSeoModal.js#L19). However, I wasn't able to get the Tag model export working properly and really wanted to fix that issue 👀 But it should have been a optional relation as well.

There are other parts in the codebase though that build upon tags, so I think the way to go here is add flarum/tags as an optional dependency to make sure the order of initialization is correct.

Hmm that's a good point, but if I remember correct, the SEO-tags are starting to load as soon the content is being rendered, so at that point everything should be initialized already right? 🤔 https://github.com/v17development/flarum-seo/blob/master/extend.php#L39

I would also suggest making use of the Conditional Extender and cleaning up extend.php a little.

I think that would be great, but I would suggest doing when making the extension compatible for Flarum 2.0 (since that would be a great time to drop support for 1.x).

[DavideIadeluca]

There are other parts in the codebase though that build upon tags, so I think the way to go here is add flarum/tags as an optional dependency to make sure the order of initialization is correct.

Hmm that's a good point, but if I remember correct, the SEO-tags are starting to load as soon the content is being rendered, so at that point everything should be initialized already right? 🤔 https://github.com/v17development/flarum-

Sorry, I was referring to flarum/tags here.

My brief research in the codebase showed that any logic which interacts with flarum/tags is conditional in one way or another. Example:

flarum-seo/src/Page/TagPage.php

Lines 58 to 63 in 02fd518

	try {
	$tag = resolve(TagRepository::class)->findOrFail($tagId);
	} catch (\Illuminate\Database\Eloquent\ModelNotFoundException $e) {
	// Do nothing, no model found
	return;
	}

To make sure that v17development/flarum-seo is booted after flarum/tags (when installed) we need to add flarum/tags as an optional dependency. Otherwise there might be edge cases where the logic which interacts with flarum/tags is not executed even if the extension is installed.

I updated this PR to add flarum/tags as an optional dependency

[jaspervriends]

Ah check! Got it. Thanks for the update and explanation, I've merged it! Will tag a new version :)