Wikipedia icon indicating copy to clipboard operation
Wikipedia copied to clipboard

Fix undefined behaviour for pages with numbers as title (e.g 7)

Open phiph-s opened this issue 2 years ago • 4 comments

When requesting the page "23", it is expected to receive this page: https://en.wikipedia.org/wiki/23 Expected behavior: wiki.page("23") -> https://en.wikipedia.org/wiki/23 wiki.page(23) -> https://en.wikipedia.org/wiki/Assistive_technology

Actual behavior: wiki.page("23") -> https://en.wikipedia.org/wiki/Assistive_technology (which has ID 23) wiki.page(23) -> https://en.wikipedia.org/wiki/Assistive_technology

So there is no way to get the page for the number 23 or for example years like 1939, sine they will be treated as IDs.

It is checked whether an ID or a Title is given by isNaN(title) in utils.ts, the String "7" will be handled like a page ID. Instead check the actual type of what the function receives: return typeof title === 'string' || title instanceof String;

New behavior: wiki.page("23") -> https://en.wikipedia.org/wiki/23 wiki.page(23) -> https://en.wikipedia.org/wiki/Assistive_technology

-> Supply a number, get a page by ID, supply a string, get the page by title

phiph-s avatar Jul 02 '23 10:07 phiph-s

Accidentally added a version change to the pull request... My bad

phiph-s avatar Jul 02 '23 10:07 phiph-s

I saw in the Tests that you actually test against strings as IDs. If the behavior is wanted like this, I would at least consider adding parameters to the query that define whether the string should be treated as ID or title.

phiph-s avatar Jul 03 '23 08:07 phiph-s

Its a very interesting point @phiph-s - apologies for the late reply i was travelling last week.

You are correct in saying that currently we do consider numbers as ids particularly. Your MR makes sense to me but i think we need some more test cases. I will work on the same in the coming weekend if you are busy with something else.

Thanks for the MR @phiph-s

dopecodez avatar Jul 10 '23 16:07 dopecodez

I'm currently quite limited in time, but maybe I'm able to write some test cases. Another option would be to implement the 2 functions "pageById" and "pageByTitle", in order to not change the behavior of the current implementation of "page", as this change might break some projects that use this package.

phiph-s avatar Jul 13 '23 08:07 phiph-s