WeakRef usage (not always supported and not recommended) #14054

@wolfbeast commented on GitHub (Jan 27, 2025):

That use of WeakRef on an input doesn't even make much sense.
I'm not too familiar with writing polyfills so I don't know how easy or difficult it would be. It attaches a .deref() to the object passed into the constructor but doesn't interfere with it otherwise AFAIK.
You can polyfill this by simply using dummy functions for the API if you're married to that github module and can't make changes to it. As explained the spec is only providing hints for the garbage collector.

@wolfbeast commented on GitHub (Jan 27, 2025): That use of WeakRef on an input doesn't even make much sense. I'm not too familiar with writing polyfills so I don't know how easy or difficult it would be. It attaches a `.deref()` to the object passed into the constructor but doesn't interfere with it otherwise AFAIK. You can polyfill this by simply using dummy functions for the API if you're married to that github module and can't make changes to it. As explained the spec is only providing hints for the garbage collector.

GiteaMirror commented

@wxiaoguang commented on GitHub (Jan 27, 2025):

That use of WeakRef on an input doesn't even make much sense.

Yep, but that's from a dependency's dependency.

You can polyfill this by simply using dummy functions for the API if you're married to that github module and can't make changes to it. As explained the spec is only providing hints for the garbage collector.

I think you can add some polyfills to Pale Moon's engine, then all websites use WeakRef could benefit. For example:

@wxiaoguang commented on GitHub (Jan 27, 2025): > That use of WeakRef on an input doesn't even make much sense. Yep, but that's from a dependency's dependency. > You can polyfill this by simply using dummy functions for the API if you're married to that github module and can't make changes to it. As explained the spec is only providing hints for the garbage collector. I think you can add some polyfills to Pale Moon's engine, then all websites use WeakRef could benefit. For example: * https://github.com/jaenster/weakref-pollyfill * https://github.com/ungap/weakrefs

GiteaMirror commented

@wolfbeast commented on GitHub (Jan 27, 2025):

I absolutely understand there's a broader solution possible, but you can't plug a web-based polyfill into an application's javascript engine like that. (if only! that would simplify a lot of things with all these convenience/sugar functions that get added to ES)
I have an open issue for implementing a stub into the engine but it's not straightforward.

@wolfbeast commented on GitHub (Jan 27, 2025): I absolutely understand there's a broader solution possible, but you can't plug a web-based polyfill into an application's javascript engine like that. (if only! that would simplify a lot of things with all these convenience/sugar functions that get added to ES) I have an open issue for implementing a stub into the engine but it's not straightforward.

GiteaMirror commented

2025-11-02 11:01:30 -06:00

@wolfbeast commented on GitHub (Jan 27, 2025):

What would be the best way to report this to your dependency's dependency? because it really should not be used this way. Even the W3C TAG Design Principles group cautions strongly against them even existing and they should at most be used for extremely specific targeted situations and never make their way into generic dependencies or broadly-used libraries...
(see also https://repo.palemoon.org/MoonchildProductions/UXP/issues/1740 where this was analyzed when the question for implementation came up)

@wolfbeast commented on GitHub (Jan 27, 2025): What would be the best way to report this to your dependency's dependency? because it really should not be used this way. Even the W3C TAG Design Principles group cautions strongly against them even existing and they should at most be used for extremely specific targeted situations and never make their way into generic dependencies or broadly-used libraries... (see also https://repo.palemoon.org/MoonchildProductions/UXP/issues/1740 where this was analyzed when the question for implementation came up)

GiteaMirror commented

@TheFox0x7 commented on GitHub (Jan 27, 2025):

Probably raise an issue/PR there: https://github.com/iansan5653/dom-input-range

@TheFox0x7 commented on GitHub (Jan 27, 2025): Probably raise an issue/PR there: https://github.com/iansan5653/dom-input-range

GiteaMirror commented

2025-11-02 11:01:30 -06:00

@wolfbeast commented on GitHub (Jan 27, 2025):

Will do. Thanks

@wolfbeast commented on GitHub (Jan 27, 2025): Will do. Thanks

GiteaMirror commented

2025-11-02 11:01:30 -06:00

@wolfbeast commented on GitHub (Feb 18, 2025):

Unfortunately there has been no response from the dom-input-range dev on the issue for 3 weeks. The dev doesn't seem to be active.
I would offer a PR but I do not work in nor know TS, and have no clue how to even test any changes I would be making :(

@wolfbeast commented on GitHub (Feb 18, 2025): Unfortunately there has been no response from the dom-input-range dev on the [issue](https://github.com/iansan5653/dom-input-range/issues/7) for 3 weeks. The dev doesn't seem to be active. I would offer a PR but I do not work in nor know TS, and have no clue how to even test any changes I would be making :(

GiteaMirror commented

2025-11-02 11:01:31 -06:00

@wxiaoguang commented on GitHub (Feb 19, 2025):

Unfortunately, this is a common phenomenon in many open source projects .......... and that's why I think it could be polyfilled (https://github.com/go-gitea/gitea/issues/33407#issuecomment-2615551658) to tolerate the WeakRef usage for more sites ......

@wxiaoguang commented on GitHub (Feb 19, 2025): Unfortunately, this is a common phenomenon in many open source projects .......... and that's why I think it could be polyfilled (https://github.com/go-gitea/gitea/issues/33407#issuecomment-2615551658) to tolerate the WeakRef usage for more sites ......

GiteaMirror commented

2025-11-02 11:01:31 -06:00

@GiteaBot commented on GitHub (Mar 21, 2025):

We close issues that need feedback from the author if there were no new comments for a month. 🍵

@GiteaBot commented on GitHub (Mar 21, 2025): We close issues that need feedback from the author if there were no new comments for a month. :tea:

GiteaMirror commented

2025-11-02 11:01:31 -06:00

@wolfbeast commented on GitHub (Mar 21, 2025):

Thank you bot for not understanding that this was an open issue and the last post was a comment, not a question.
This remains an issue. Unfortunately I can't re-open this.

@wolfbeast commented on GitHub (Mar 21, 2025): Thank you bot for not understanding that this was an open issue and the last post was a comment, not a question. This remains an issue. Unfortunately I can't re-open this.

GiteaMirror commented

2025-11-02 11:01:32 -06:00

@wxiaoguang commented on GitHub (Mar 26, 2025):

Thank you bot for not understanding that this was an open issue and the last post was a comment, not a question. This remains an issue. Unfortunately I can't re-open this.

TBH I don't want to disappoint you, but actually it is not Gitea's problem ..... If it is only Gitea's problem, we could polyfill it in Gitea's code base (We have done so: #28441, #26575, #23592, etc and IMO it's better to avoid polyfills because many of them are still incomplete)

However, many sites including GitHub also use WeakRef (that JS library is also used by GitHub), so if you don't introduce a general WeakRef polyfill in the browser engine, many sites are still broken.

@wxiaoguang commented on GitHub (Mar 26, 2025): > Thank you bot for not understanding that this was an open issue and the last post was a comment, not a question. This remains an issue. Unfortunately I can't re-open this. TBH I don't want to disappoint you, but actually it is not Gitea's problem ..... If it is only Gitea's problem, we could polyfill it in Gitea's code base (We have done so: #28441, #26575, #23592, etc and IMO it's better to avoid polyfills because many of them are still incomplete) However, many sites including GitHub also use WeakRef (that JS library is also used by GitHub), so if you don't introduce a general WeakRef polyfill in the browser engine, many sites are still broken.

GiteaMirror commented

2025-11-02 11:01:32 -06:00

@wolfbeast commented on GitHub (Mar 26, 2025):

I know it's a dependency of a dependency issue. But neither the dependency nor the dependency of the dependency repo maintainers are responding to any sort of attempted contact. If they would, it could be solved for anyone using that dom-range module. Ultimately that means it becomes a Gitea problem because you chose to rely on this (clearly unmaintained) dependency, which unnecessarily attempts to reach deep into the js machinery with WeakRef for no other reason than it "being a thing".
It's not like I didn't try to get the root issue fixed. I have.

I am well aware that it would be preferable to implement at least a stub WeakRef in the browser (which would be there just to satisfy use without fallback of WeakRef and not do anything otherwise) and that is on our to-do list but it is proving a lot more complicated than it should be because of the complexities of our inherited SpiderMonkey engine.
Of note also is that GitHub doesn't have this issue Gitea has (I have no trouble getting a proper pop-up on issue mention that works and is clickable, etc. which is broken because of WeakRef in Gitea) and Gitea has been the only web GUI so far I've encountered that breaks on it not being present. "many sites" is an overstatement.

@wolfbeast commented on GitHub (Mar 26, 2025): I know it's a dependency of a dependency issue. But neither the dependency nor the dependency of the dependency repo maintainers are responding to any sort of attempted contact. If they would, it could be solved for anyone using that dom-range module. Ultimately that means it _becomes_ a Gitea problem because you chose to rely on this (clearly unmaintained) dependency, which unnecessarily attempts to reach deep into the js machinery with WeakRef for no other reason than it "being a thing". It's not like I didn't try to get the root issue fixed. I have. I am well aware that it would be preferable to implement at least a stub WeakRef in the browser (which would be there just to satisfy use without fallback of WeakRef and not do anything otherwise) and that is [on our to-do list](https://repo.palemoon.org/MoonchildProductions/UXP/issues/2689) but it is proving a lot more complicated than it should be because of the complexities of our inherited SpiderMonkey engine. Of note also is that GitHub doesn't have this issue Gitea has (I have no trouble getting a proper pop-up on issue mention that works and is clickable, etc. which is broken because of WeakRef in Gitea) and Gitea has been the only web GUI so far I've encountered that breaks on it not being present. "many sites" is an overstatement.

GiteaMirror commented

@wxiaoguang commented on GitHub (Mar 26, 2025):

OK, maybe GitHub also polyfills.

Could you try whether this PR work?

-> Polyfill WeakRef #34025

@wxiaoguang commented on GitHub (Mar 26, 2025): OK, maybe GitHub also polyfills. Could you try whether this PR work? -> Polyfill WeakRef #34025

GiteaMirror commented

@wolfbeast commented on GitHub (Mar 26, 2025):

Could you try whether this PR work?

I'd love to, but I'm neither familiar with your build process nor set up to do any golang testing. Is there a way I can slot this into a deployed Gitea instance to test?

@wolfbeast commented on GitHub (Mar 26, 2025): > Could you try whether this PR work? I'd love to, but I'm neither familiar with your build process nor set up to do any golang testing. Is there a way I can slot this into a deployed Gitea instance to test?

GiteaMirror commented

@wxiaoguang commented on GitHub (Mar 26, 2025):

Hmm, I can test it locally (and have tested it locally) and then after it gets approved, then there will be a nightly built.

@wxiaoguang commented on GitHub (Mar 26, 2025): Hmm, I can test it locally (and have tested it locally) and then after it gets approved, then there will be a nightly built.

GiteaMirror commented

@wolfbeast commented on GitHub (Mar 26, 2025):

OK let me know when you have a nightly build that I can drop into production to test, and I'll verify. Thanks!

@wolfbeast commented on GitHub (Mar 26, 2025): OK let me know when you have a nightly build that I can drop into production to test, and I'll verify. Thanks!

GiteaMirror commented

@silverwind commented on GitHub (Mar 26, 2025):

According to https://github.com/jaenster/weakref-pollyfill/issues/1 WeakRef is not polyfillable, so while our pseudo-polyfill might works, it'll probably never act like the real thing, because JS does not expose the necessary garbage collector interfaces for it to be polyfillable.

@silverwind commented on GitHub (Mar 26, 2025): According to https://github.com/jaenster/weakref-pollyfill/issues/1 WeakRef is not polyfillable, so while our pseudo-polyfill might works, it'll probably never act like the real thing, because JS does not expose the necessary garbage collector interfaces for it to be polyfillable.

GiteaMirror commented

@wxiaoguang commented on GitHub (Mar 26, 2025):

According to jaenster/weakref-pollyfill#1 WeakRef is not polyfillable, so while our pseudo-polyfill might works, it'll probably never act like the real thing.

It won't be a problem, in our case, no memory management requirement, and actually the package using WeakRef seems abusing it (according to the discussions above).

@wxiaoguang commented on GitHub (Mar 26, 2025): > According to [jaenster/weakref-pollyfill#1](https://github.com/jaenster/weakref-pollyfill/issues/1) WeakRef is not polyfillable, so while our pseudo-polyfill might works, it'll probably never act like the real thing. It won't be a problem, in our case, no memory management requirement, and actually the package using WeakRef seems abusing it (according to the discussions above).

GiteaMirror commented

@wolfbeast commented on GitHub (Mar 26, 2025):

According to https://github.com/jaenster/weakref-pollyfill/issues/1 WeakRef is not polyfillable

The front-end most definitely is, and that is all that matters. You shouldn't expect a polyfill to take over what is handled by GC internals, but that is the whole point of why it shouldn't actually be a thing to begin with. JS engines will always know much better when and how to collect garbage than any cross-browser, web-based content would. WeakRef makes no guarantees, and there are critical notes from everyone involved it shouldn't be relied on for memory management. It only provides hints that a script "would like something to be GCed", but whether it, in fact, is, depends entirely on the state of the JS engine internals. It may happen later or not at all. Providing a stub is therefore just fine.

@wolfbeast commented on GitHub (Mar 26, 2025): > According to https://github.com/jaenster/weakref-pollyfill/issues/1 WeakRef is not polyfillable The front-end most definitely is, and that is all that matters. You shouldn't expect a polyfill to take over what is handled by GC internals, but that is the whole point of why it shouldn't actually be a thing to begin with. JS engines will always know much better when and how to collect garbage than any cross-browser, web-based content would. WeakRef makes no guarantees, and there are critical notes from everyone involved it shouldn't be relied on for memory management. It only provides hints that a script "would like something to be GCed", but whether it, in fact, is, depends entirely on the state of the JS engine internals. It may happen later or not at all. Providing a stub is therefore just fine.

GiteaMirror commented

@wxiaoguang commented on GitHub (Mar 27, 2025):

1.2 nightly is ready

@wxiaoguang commented on GitHub (Mar 27, 2025): 1.2 nightly is ready * https://dl.gitea.com/gitea/1.23-nightly/ * https://hub.docker.com/r/gitea/gitea/tags?name=1.23-nightly

GiteaMirror commented

@wolfbeast commented on GitHub (Apr 3, 2025):

Sorry for the delay in answering - I can confirm the nightly build solves the issue and that the polyfill works as it should.

@wolfbeast commented on GitHub (Apr 3, 2025): Sorry for the delay in answering - I can confirm the nightly build solves the issue and that the polyfill works as it should.

GiteaMirror commented

@wxiaoguang commented on GitHub (Apr 3, 2025):

Glad to help.

ps: I can't access repo.palemoon.org, it always redirects to https://repo.palemoon.org/assets/geoblock.html

I guess this PR might help you: Add a config option to block "expensive" pages for anonymous users #34024 (background: #33966): when set REQUIRE_SIGNIN_VIEW=expensive, many pages will require sign-in to block AI crawlers.

@wxiaoguang commented on GitHub (Apr 3, 2025): Glad to help. ps: I can't access `repo.palemoon.org`, it always redirects to `https://repo.palemoon.org/assets/geoblock.html` I guess this PR might help you: `Add a config option to block "expensive" pages for anonymous users` #34024 (background: #33966): when set `REQUIRE_SIGNIN_VIEW=expensive`, many pages will require sign-in to block AI crawlers.

GiteaMirror commented

@wolfbeast commented on GitHub (Apr 3, 2025):

Yeah I was forced to blanket geoblock certain regions because of IP-spread hammering of our repo with crawling requests going through all tag combinations (requesting the same pages over and over and over...) causing 1500% CPU load which our host wasn't happy about ;P
I'll try the option and see how that goes. Thanks for the tip!

@wolfbeast commented on GitHub (Apr 3, 2025): Yeah I was forced to blanket geoblock certain regions because of IP-spread hammering of our repo with crawling requests going through all tag combinations (requesting the same pages over and over and over...) causing 1500% CPU load which our host wasn't happy about ;P I'll try the option and see how that goes. Thanks for the tip!

GiteaMirror commented

@wolfbeast commented on GitHub (Apr 3, 2025):

REQUIRE_SIGNIN_VIEW=expensive

Unfortunately this won't work for us. requiring an account+signin to even view commits just isn't good practice for FOSS.
Also, seems you should really add "/compare" to it; they are some of the most expensive calls in gitea.

@wolfbeast commented on GitHub (Apr 3, 2025): > REQUIRE_SIGNIN_VIEW=expensive Unfortunately this won't work for us. requiring an account+signin to even view commits just isn't good practice for FOSS. Also, seems you should really add "/compare" to it; they are some of the most expensive calls in gitea.

GiteaMirror commented