4chan-x
4chan-x copied to clipboard
ccd0
Migrate to JavaScript/TypeScript
Fixes #829
Creating a draft PR because this is not entirely done, but I think it's far enough to discuss. There was already some discussion on /g/, but I think it's better to have the discussion here.
This migration is based on transpilation done with decaffeinate, with export/import syntax added. Bundling is now done with rollup, and can be done with npm i; npm run build, and not with make anymore.
I tried to keep it at close as possible to decaffinates output, but I had do do some things to make it compatible with the new build and get it running without errors, like finding an alternative to the old custom templates, which is now done with jsx.
This build is still mostly .js, but the build allows simply renaming files .js to .ts or .tsx except for build scripts. Typing every file would be an ever bigger project than this migration, so for the foreseeable future types can be added when needed or wanted.
notes
- A lot of files have circular dependencies, but rollup can handle that
- but for some scripts that add to the same object I had to merge them, like Posting/QR and site/SW.yotsuba.js
- sometimes something might not be initialized before use, for example,
$.dict()and$.SECONDS- I moved these to a new file called helpers.ts, which shouldn't have dependencies itself, so it's always available
- tsconfig.json has
"checkJs": true,, and a lot of js files report type errors when opened because of unknown properties on objects and reassigning variables with different types. These errors don't block the bundle at this moment. - old files in the builds directory stay as reference until the new builds are have the complete output, new files go in the builds/test directory until then
- old build scripts are also kept around for reference until the new build output is fully functional
- the es 2020 target was chosen for optional chaining, a feature often used in coffeescript
- minimum browser version needed to be bumped for this
- @violentmonkey/types was chosen over @types/greasemonkey because @types/greasemonkey only declares the GM object, and not GM_ functions
- ~~I don't really understand PostClone, so that might have bugs after migration~~ I think the main thing that confused me the the workaround coffeescript put in to not run the constructor of the parent class Post. A es class compatible workaround was implemented.
Things that still need to be done:
- [x] is <% if (readJSON('/.tests_enabled')) { %>, still used? Should it be functional or should it be removed? There are not a lot of tests for such a big project
- [ ] actual structure of the build uitput. I got a userscript and a unpacked extension, but I have trouble reading the make file, so I don't know the full intention
- ~~add things to the global window for use by other userscripts~~ according to https://github.com/ccd0/4chan-x/pull/3341#issuecomment-1456805345, non global is intended
- [ ] port updates made to 4chan-X since this was forked
- [ ] clean up old build scripts
- [ ] update docs
- [ ] thorough testing, this conversion might have broken things
commits since this was forked
Click to expand
- [x] 944b04210c119aedf8da1a8bcabaca9b80312118 Update archive list.
- [x] 59ee8c57792d0f82491756a077e25f506fd62994 Desuarchive removes /gif/
- [x] 402679e33a06dfbe0dc39ceba5c24fed761b6a19 desuarchive removes /wsg/ files
- [x] 86071184aa39b3585f06c1a4e2921c411ad8cf10 archived.moe adds /pw/ search, tokyochronos has hosting issues
- [x] 8a6392b1cf721ddfae6d8f4e3ec2566f15755370 add Eientei
- [x] 451a06f54b878ce433b0775858affefc71927fc7 alice.al domain change
- [x] 2a8bf2adb0737ce7bb1e21f6b959e4c6e1de1bc7 Disable Javascript Whitelist on captcha iframe. #3292
- [x] e9c1529da7844a42a1b40458c2c77b77e23ca537 Make QR post more like original form post. #3330
- [x] d16062a8fac5c092c34310c7704ac3980494b6ef Merge remote-tracking branch '4chenz/master'
- [x] 8795b1c56dbdfb52a32ddb3ea80b549f0048dc7b Add Google Lens image search url
- [x] f3f03f5e79fb5f26c0fd4406b2ab6796851ea471 Replace Google image search link with Google Lens.
- [x] c68a8afbdf30e3cbb35f0834b364f20600151adf Switch Google image search back to old version, thanks to https://boards.4channel.org/g/thread/91737566#p91789527
- [x] aef984da1a6af4d0003b51e7f03bce252ac71dff Remove empty space from ads if they don't load. https://kissu.moe/b/res/7155#11052
- [x] 19268975ea2d49a753624315b0928f27496aac02 Update Randomize Filename to match current 4chan format. https://boards.4channel.org/g/thread/91737566#p91784238
- [x] 2a47dfd8ba724b17f5bc5f9214bea8ce8b469398 Catch errors due to "Restricted" selection. #2905
- [x] 27957c25af5d182adc38f1e67a098ab338631ccd Release 4chan X v1.14.22.2.
- [x] eb25d6e797a1673fd7cddb257fce04055383ec9b Update chrome-webstore-upload.
- [x] 14e67e9a958633e37b4e4a6293cfa3a921c1eab0 Release 4chan X v1.14.22.3.
- [x] 7295b21b73eb13ec53fdc61767ada341c2e13144 Avoid breaking sauce settings of people with links to original Google Images and Google Lens, provided they didn't already update to v1.14.22.3.
- [x] 71873cd7b22a565c2a41fa24f63f7504152683eb Recognize JPEG files with .jfif extensions as images for purposes of Image Hover etc.; also recognize .avif and .jxl files as images.
- [x] ea2462ecc47327c6f0c31348d95fd2b1b6447cb3 Release 4chan X v1.14.22.4.
nice to have in the future
- [ ] get rid of all circular dependencies
- [ ] rewrite functions using
.callto set thethisobject to passing variables. Unlessthisis explicitly typed, it won't be recognized by TypeScript and listed in references or when renaming variables across the project - [ ] look at the decaffeinate suggestions added to files
Based of you to do this.
I cloned the repo and build the Userscirpt my self
I found out that alot of the bugs are due to the type of " ' ` used
If you look at the code with fx violentmonkey you can see it for your self see pic rel
I will try to look trough the files and try to fix them, but it might be a little difficult due to my own fork. ( I think i will just make a burner/second acc )
As for all the things that you find strange, weird, etc like the Makefile, "Json test thing" , etc is that you just delete or rewrite it. I mean it uses Coffee script, a language that only shows 12 projects when searching it up on Github most of them being left to rot, the reason for the rewrite is to remove all the obscure shit and bloat
@LalleSX
I found out that alot of the bugs are due to the type of " ' ` used
Seems to be a problem with violentmonkeys syntax highlighting. Doesn't happen when in VS Code, or in GitHub markdown:
$$1.extend(link, {
innerHTML:
`<a href="javascript:;" class="qr-link">${g.VIEW === "thread" ? "Reply to Thread" : "Start a Thread"}</a>`
});
Having an unclosed ` would be a syntax error that crashes the whole script, which isn't the case on my machine.
I have tried building this myself and i have found a bug: When useing the 4chans own 3 day Archive and going on a post it gives an error:
"Normalize URL" initialization crashed. SecurityError: The operation is insecure.
(4chan XT TuxedoTako v1.14.22.1 userscript on gecko at 27656,28989,24915,28891,23617,28870,24258,28865,29605,23349)
I will look and try to fix this at this myself, I will report back if i fix it.
Fixed it Here is the commit on my bransh
Maybe you all ready know this, but when bug fixing try to look at the original 4chan-X build files, that is what helped me here.
is <% if (readJSON('/.tests_enabled')) { %>, still used?
The most important test is checking if the HTML built from JSON matches 4chan's HTML. This should probably be rewritten to happen in a script running in Node that imports a few relevant modules instead of in a special build of the userscript.
@LalleSX
Fixed it Here is the commit on my bransh
Thanks, can you open a to my fork? Preferably the 'project-XT' branch.
@cc0
is <% if (readJSON('/.tests_enabled')) { %>, still used?
The most important test is checking if the HTML built from JSON matches 4chan's HTML. This should probably be rewritten to happen in a script running in Node that imports a few relevant modules instead of in a special build of the userscript.
That's probably important now that I changed the html generation. I'll see if I can put it in a separate script.
The test rely on the dom, same with the things it's testing. Making them a node script would not only mean rewriting the tests, but a lot of the script as well.
Ran the tests. Only differences with the old template output was a few spaces between elements. I changed the script to make the tests pass.
@TuxedoTako I made a pull req https://github.com/TuxedoTako/4chan-xt/pull/1
Also @ccd0 i think a history/log wipe is needed after this merge ( An archive can be be but elsewhere) Because it's really unnecessarily big
I think its because of multiple years of having multiple build files (aka user scripts ) in the source code
Tagging is a lot better option and github supports having binaries/files on tags under releases.
or at least only have one file instead of 3.
The history is also no longer important due to the language change
@LalleSX It is the builds, specifically the compressed .crx/.zip files rather than the userscripts. I've been planning to do a history rewrite to remove the builds and put the builds in their own separate repository.
@LalleSX I asked for a PR of the fix, I didn't expect you to start working on the build script, moving some files, and emptying the build directory. For that I have to ask @ccd0 If he's ok with that, since those changes will also land in this PR to his repo.
Now that I'm pinging anyway, are there more documented behaviors of 4chan-X for other scripts extending it than the events listen on the GitHub wiki? I don't know what needs to be added to the global window to make other scripts work, like 4chan sounds player, which I use myself and doesn't work with the fork. Or is this a case of scripts relying on observed behaviour, meaning things like replacing Object.create(null) with new Map() might be a breaking change in the future?
I would assume the switch to TS is one PR, and the changing of make/build is another, and the history stuff/deleting old builds is another.
I think most userscripts utilise the API provided in the wiki along with things like IndexRefresh and IndexBuild. Otherwise personally I use MutationObservers in addition for when things don’t fire as you expect. There is an issue somewhere about adding more like PostsAdded as an array within threads.
As saxamaphone69 said, breaking this stuff into multiple PRs makes sense. Also it looks like @LalleSX deleted a bunch of stuff from the Makefile but didn't write replacement functionality, mainly the stuff about updating the webpage and pushing builds to 4chan-x.net. That stuff would need to be replaced.
For security reasons, the only way userscripts can communicate each other is by firing events or updating the page. So there's no need to worry about replacing Object.create(null) with new Map() internally unless it's an object that gets sent to another script via an event.
I would assume the switch to TS is one PR, and the changing of make/build is another
Because the old build script expects coffeescript, it can't build the new version, so I had to make a new build script along with the migration to make sure it runs.
and the history stuff/deleting old builds is another
It was never my intention to put that in this PR.
Also it looks like @LalleSX deleted a bunch of stuff from the Makefile but didn't write replacement functionality.
Yep, like I mentioned in the op or the readme of my fork, I wanted to at least keep the old build scripts until the new one can do everything the old one did. Sorry Lalle, but I can't merge a PR that removes old functionality without writing a new alternative. Especially since the script that updates the web page might not even need a rewrite. If I thought just outputting an userscript and extension directory was enough this PR would be out of draft.
For security reasons, the only way userscripts can communicate each other is by firing events or updating the page.
I should look deeper into the 4chan sounds player script and what is actually breaking it.
Looks like I may need to rewrite some of the build and distribution stuff prior to this pull request being merged just in order for contributors to make releases. Will post more details in #3339.
It may be some time before we stop updating the CoffeeScript version, so manually porting the code might not be sustainable. Have you tried instead running decaffeinate on the updated version, then merging that with your branch?
Because the old build script expects coffeescript, it can't build the new version,
This isn't actually the case. If you just change the names to .js, they should work with the old build script. A few files are already in Javascript.
It may be some time before we stop updating the CoffeeScript version, so manually porting the code might not be sustainable. Have you tried instead running decaffeinate on the updated version, then merging that with your branch?
Decaffeinate didn't add the imports and exports, that was me. So a new decaffeinate will be merge conflict hell.
I already ported up to release v1.14.22.4, and I'm willing to continue that for a while if the changes aren't too big.
Because the old build script expects coffeescript, it can't build the new version,
This isn't actually the case. If you just change the names to .js, they should work with the old build script. A few files are already in Javascript.
Looks like my new import/export version broke that...
import/export was needed for TypeScript anyway, not just for .ts files, but for nice IDE functions like going to definition and looking up references. The alternative is declaring globals manually.
Anyway, I have trouble understanding the Makefile, and since it apparently does more than building the script, it seems that it can't be replaced with my npm run build like I initially thought. Can somebody help me with integrating the new build in the Makefile?
Found one problem with the 4chan sound player: https://github.com/rcc11/4chan-sounds-player/blob/f2953446fd9403872642e05f91f580aab148ec44/src/main.js#L35. Because it's designed to work with both 4chan-X and native, it also listens on DOMContentLoaded, which 4chan-X is also waiting for before initializing, resulting in a race condition.
Is the src/platform/$ supposed to be a like Jquery script?
If yes then mabye its time to use real Jquery with
// @require http://code.jquery.com/jquery-latest.js
An example/showcase of this working is this:
function main() {
alert("There are " + $('a').length + " links on this page.");
}
main();
use jquery
The bloat is not needed in modern browsers. It's just jQuery-like as in aliased in an easier-to-manage way and chainable.
node_modules/.bin/decaffeinate --loose --nullish-coalescing --optional-chaining src/*/*.coffee
seems to reproduce most of it except for commenting out the constructor in Post.Clone; also the suggestion "DS002: Fix invalid constructor" seems to have been removed. Was that done manually?
A note about file history
You may notice that bulk-decaffeinate generates multiple commits instead of just one. In particular, it renames the file in one commit and changes the contents in a separate commit. This allows git to properly track file history across the conversion.
For example, once you've converted your file, you can run git log --follow -- path/to/file.js and see the history of the .coffee file too.
Probably a good idea to split up the decaffeinate commit into two parts like this. That way it's still easy to look at the history of a particular file. Subsequent commits would be rebased on that.
Regarding archive.json, that's something I periodically pull from https://github.com/4chenz/archives.json so it should be left as a plain JSON file if possible. Otherwise, there will need to be a conversion every time I pull it.
Why does this commit cca085e60090ca21edf0dee6aa012fc4c949809a have so many whitespace changes?
Decaffeinate didn't add the imports and exports, that was me. So a new decaffeinate will be merge conflict hell.
I already ported up to release v1.14.22.4, and I'm willing to continue that for a while if the changes aren't too big.
Tried it myself, and even with the whitespace changes, merging doesn't seem any harder than porting the code, and is probably more reliable. (In your port of the settings updates, the regexp /\bsbisrc=/ was erroneously changed to /\bsbisrc = /, and you forgot the bit that adds a commented lens link.) The biggest annoyances not due to whitespace changes were files that had been split/renamed, but probably a smarter merge method can deal with that.
It could be a year before I switch the stable version from CoffeeScript to JavaScript, so it will probably be me merging commits at that point.
It might also reduce diff noise to leave the d and doc abbreviations as they are, and simply declare them at the top of the files that use them. But I don't know if that causes problems for autocompletion. I should probably test out the IDE you use on this project.
I'm still not convinced the JSX thing is a good idea. It looks like it adds needless complexity relative to just using templates, and the fact that it's not quite standard JSX is another point against it.
What options did you use for decaffeinate?
I forgot...
node_modules/.bin/decaffeinate --loose --nullish-coalescing --optional-chaining src//.coffee
seems to reproduce most of it except for commenting out the constructor in Post.Clone; also the suggestion "DS002: Fix invalid constructor" seems to have been removed. Was that done manually?
Yes, I wanted to at least get the code to run before creating a PR.
I must admit I made other changes suggested by decaffeinate, like removing Array.from around things that were already arrays, and changing edits to class prototypes to static properties.
A note about file history
You may notice that bulk-decaffeinate generates multiple commits instead of just one. In particular, it renames the file in one commit and changes the contents in a separate commit. This allows git to properly track file history across the conversion.
For example, once you've converted your file, you can run git log --follow -- path/to/file.js and see the history of the .coffee file too.
Probably a good idea to split up the decaffeinate commit into two parts like this. That way it's still easy to look at the history of a particular file. Subsequent commits would be rebased on that.
I did to much work outside the decaffeinate, like the imports/exports, build, fixes to get get that build running, etc, to do it again. I can try a rebase on top of that. Maybe squash some work in progress commits.
What are your thoughts on rewriting the history of this PR?
Why does this commit https://github.com/ccd0/4chan-x/commit/cca085e60090ca21edf0dee6aa012fc4c949809a have so many whitespace changes?
Auto formatting. I didn't expect you to go trough this commit by commit, and the contributing guidelines don't specify formatting rules, so I don't expected that to be a problem.
You can add ?w=1 to a github url to hide white space changes to a diff.
Regarding archive.json, that's something I periodically pull from https://github.com/4chenz/archives.json so it should be left as a plain JSON file if possible. Otherwise, there will need to be a conversion every time I pull it.
Fair. I started converting .json files to js at the start, but the wrote custom code to import the package.json and version.json, so I should probably do that for other files that were previously .json.
the regexp /\bsbisrc=/ was erroneously changed to /\bsbisrc = /
Weird, the auto formatter should know regexes.
It might also reduce diff noise to leave the d and doc abbreviations as they are, and simply declare them at the top of the files that use them.
Like I said, om not doing the decaffeinate a second time. Maybe I can revert those manually and squash those in the commit from the bulk decaffinate.
I'm still not convinced the JSX thing is a good idea. It looks like it adds needless complexity relative to just using templates,
I wouldn't call the original src/site/SW.yotsuba.Build/PostInfo.html simple.
and the fact that it's not quite standard JSX is another point against it.
What do you mean standard JSX? React? Which version? There is a specification that facebook published: https://facebook.github.io/jsx/ that's just a guide on how to transpile it. I tried to stay close to html strings, like how I'm not changing attribute names in the functions.
There was talk about tagged templates in a /g/ thread, but I don't think it would be much simpler, especially when porting what was ?{. Maybe for simpler things than PostInfo.html? But then we end up with two escape functions.
I did to much work outside the decaffeinate, like the imports/exports, build, fixes to get get that build running, etc, to do it again.
Presumably most of that was in separate commits though? It looks like it.
I can try a rebase on top of that. Maybe squash some work in progress commits. What are your thoughts on rewriting the history of this PR?
If it makes things clearer, feel free to rewrite the history if you want to. If you can get rid of the whitespace changes in cca085e60090ca21edf0dee6aa012fc4c949809a, that would be good. But another commit reverting them would also work.
Like I said, om not doing the decaffeinate a second time. Maybe I can revert those manually and squash those in the commit from the bulk decaffinate.
You don't need to do it, I'll be doing it. So to avoid interfering with the merges I'm going to be doing, can you try to avoid unnecessary diff noise between the decaffeinate commit (which should be reproducible just by me running decaffeinate) and the final pull request? This would mean not including ported commits in the pull request.
Oh, and please don't squash in even more changes with the bulk decaffeinate commit, that would make things much worse.
I wouldn't call the original src/site/SW.yotsuba.Build/PostInfo.html simple.
The old templating system has a lot of complexity, but that complexity isn't at runtime. If you could get the JSX stuff to compile to something simple at build time, instead of having to walk a tree of elements at runtime, that would be a lot more attractive.
What do you mean standard JSX? React? Which version?
I don't know much about JSX, but I presumed the main advantage of using it would be that people familiar with it will know how to use it without us having to explain it much. But it seems there are subtle differences like the class/className thing. Not really a dealbreaker, just a minor point.
There was talk about tagged templates in a /g/ thread, but I don't think it would be much simpler, especially when porting what was ?{. Maybe for simpler things than PostInfo.html? But then we end up with two escape functions.
Looks like you're dealing with it by pushing the items into an array, which seems reasonable. So the tagged template function just needs to be able to deal with array-valued placeholders, in addition to safe HTML and unsafe strings. Maybe I'll write it myself and we can see what the runtime performance is like compared to the JSX version.
I pushed a decaf2 branch; the files at 98dcd7c are identical to your bc163cd with the exception of the Post.Clone constructor being commented out. You can make that change in a separate commit and then it should be easy to rebase everything subsequent to bc163cd on top of that.