search

flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.27k stars 826 forks source link

typo in js part of sticky plugin #3886

Closed TsXor closed 10 months ago

TsXor commented 11 months ago

Current Behavior

https://github.com/flarum/sticky/blob/main/js/src/forum/addStickyExcerpt.js#L9

Steps to Reproduce

open that link

Expected Behavior

that function name should be addStickyExcerpt

Screenshots

No response

Environment

LNMP

Output of php flarum info

No response

Possible Solution

rename it

Additional Context

This have't caused problems, but idk why.

TsXor commented 11 months ago

also, in code of sticky plugin, "sticky" can mean "a state of being sticky" or "make sticky", which may add some ambiguity

TsXor commented 11 months ago

https://discuss.flarum.org/d/33369-weird-js-issue-relating-to-extensions-typeerror-t-is-undefined/2 seems to be related to this. Look at the error: we can see in the exception stack: loadPage DiscussionListState.ts:43 and when we take a look at addEssentialExcerpt.js, we will find that only this file in Sticky plugin imported from DiscussionListState.ts

Also, the function addEssentialExcerpt makes pinned discussions show a 175-char abstract, which means that its missing can stop the page from loading.

Well, all the above things seems to make sense until I tried a compilation. forum.js didn't even change, which means that the typo is harmless.

SychO9 commented 11 months ago

the typo is harmless, whatever error is caused in that report, it is most likely due to an extension.

clarkwinkelmann commented 11 months ago

If there was really a problem with the naming of the import, the entire extension would fail to compile before it's ever released publicly.

Most imports in Flarum javascript simply take the default export and give it a name that matches the file so the actual function name in the export rarely matters.

Still something we might want to rename internally, I assume it's a leftover from an older rename.

SychO9 commented 10 months ago

the function was renamed in 2.0 but other then that, nothing to fix here.