Attempt to get publication year when auto-titling links

[rleed]

To resolve #51.

[ekzyis]

Regarding:

I would put this logic into getMetadata using a custom ruleset. Afaict, looking at the code, you can add new rules (like for date here) and not only extend existing rules. Just add a new key to the ruleset.

I see you are already using getMetadata.

Why are you extracting the date in different functions? Why not have a single call to getMetadata with all date rules? You don't have to reimplement the python code 1:1.

You also mention this:

// try to get date from various sources in order of precedence

But the rule order would already specify the precedence:

The order in which rules are defined indicate their preference, with the first rule being the most preferred.

-- https://www.npmjs.com/package/page-metadata-parser#rules

What's interesting is that there was already a PR for this but the package is unmaintained since February 2022: https://github.com/mozilla/page-metadata-parser/pull/122/files

I haven't found a maintained fork or similar though.

I think we should also use the name publishedDate instead of just date for the rule because it's more clear what kind of date is meant (even though I don't know what other date could be meant, lol).

Also, do you have example websites where different tags are used for the published date? The url in your comment uses script[type="application/ld+json"] but would be nice to see examples for the other tags, too.

edit: also please do a rebase, there are conflicts currently

[rleed]

Just a quick update that I've been working on this. I probably won't have new push till next week though.

[ekzyis]

Mhh, I think we missed that this is ready for review again.

I'll try to do a review tomorrow.

[ekzyis]

Looks much better now!

Most important comments:

• I don't understand the purpose of initDateRule
• TypeError for links where no publication date was found

[ekzyis]

By the way, it's a good practice to rebase your branches frequently on master to avoid complex conflicts.

There are currently no conflicts but this means you can rebase for free.

For example, your branch date-scraper is 49 commits behind master while date-ranges and your master branch are 42 commits behind:

$ git rev-list master --not rleed/date-scraper | wc -l
49
$ git rev-list master --not rleed/date-ranges | wc -l
42
$ git rev-list master --not rleed/master | wc -l
42

[rleed]

• I don't understand the purpose of initDateRule

It just adds our custom rule to the ruleset, since it isn't built-in. Without it, the following line wouldn't know how to find the date in the page. If there's a better way/place to do it, I'm open, but this just seemed like the most logical/reliable place to put it.

[rleed]

By the way, it's a good practice to rebase your branches frequently on master to avoid complex conflicts.

There are currently no conflicts but this means you can rebase for free.

For example, your branch date-scraper is 49 commits behind master while date-ranges and your master branch are 42 commits behind:

$ git rev-list master --not rleed/date-scraper | wc -l
49
$ git rev-list master --not rleed/date-ranges | wc -l
42
$ git rev-list master --not rleed/master | wc -l
42

Thanks... I will go through them all now.

[ekzyis]

It just adds our custom rule to the ruleset, since it isn't built-in. Without it, the following line wouldn't know how to find the date in the page. If there's a better way/place to do it, I'm open, but this just seemed like the most logical/reliable place to put it.

Oh, I never added why I don't understand it. I had written a lengthy comment about it but somehow I must have lost it in all my tabs, lol. Github is confusing if you have multiple tabs of it open ...

I don't understand it because you're manipulating something you imported at lib/timedate-scraper.js:95:

metadataRuleSets.publicationDate = ruleSet

I am actually surprised this even works - I guess because javascript imports are references? So this changed value is actually reflected where you use it in api/resolvers/item.js:489:

const metadata = getMetadata(doc, url, { title: metadataRuleSets.title, publicationDate: metadataRuleSets.publicationDate })

But since initDateRule takes no arguments ... why don't you just export ruleSet? And then import that in api/resolvers/item.js:489? That's beyond me.

Anyway, you shouldn't manipulate imports imo. That probably relies on very specific ESM import mechanics and I've never seen this before, lol

[rleed]

Done, thanks!