Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 'context' parameter from TGUIs #12172

Merged
merged 4 commits into from
Jan 23, 2025

Conversation

itsmeow
Copy link
Member

@itsmeow itsmeow commented Jan 19, 2025

About The Pull Request

Ports tgstation/tgstation#80003

For maintainers, the best way to review this is to just check the first commit on this PR for the actual technical changes. Everything else is just automatic regex.

Regex For Downstreams

I wrote these myself. They "just work". Don't question it.

In folder: tgui/packages/tgui/interfaces/**


REPLACE: useBackend(<\w+>)?\(([\n ]*?)?(this\.)?context\s?\)
WITH: useBackend$1()


REPLACE: use(Local|Shared)State(<[\S\s]+>)?\(([\n ]*?)?(this\.)?context,? ?
WITH: use$1State$2(


REPLACE: \(([\n ]*?)?((\w+|\{([\S\s]+\}))(: (\w+|\{([\S\s]+\})))?),([\n ]*?)?_?context(: (\w+|\{([\S\s]+\})))?([\n ]*?)?\)
WITH: ($2)

How to use

image

Why It's Good For The Game

This is a precursor for React. The 'context' parameter isn't used in modern React. There's no need to pass it around a bunch.

Testing Photographs and Procedure

Screenshots&Videos

TGUIs work.

image

image

image

Changelog

🆑 itsmeow, jlsnow301
code: Removed 'context' parameter from TGUIs. This is an internal change that should not affect anything.
/:cl:

jlsnow301 and others added 3 commits January 19, 2025 01:35
*Most context
This is a precursor PR for moving to React. Newer React (>16) does not
have the `context` parameter, so it needs removed. I want to make
converting to React look slightly less intimidating, so this is just
short-circuiting context here.

This shouldn't be considered a long term solution.
Moving to react is good actually
N/A hopefully no noticeable change
In folder: tgui/packages/tgui/interfaces/**

REPLACE: useBackend(<\w+>)?\(([\n ]*?)?(this\.)?context\s?\)
WITH: useBackend$1()

REPLACE: use(Local|Shared)State(<[\S\s]+>)?\(([\n ]*?)?(this\.)?context,? ?
WITH: use$1State$2(

REPLACE: \(([\n ]*?)?((\w+|\{([\S\s]+\}))(: (\w+|\{([\S\s]+\})))?),([\n ]*?)?_?context(: (\w+|\{([\S\s]+\})))?([\n ]*?)?\)
WITH: ($2)
@github-actions github-actions bot added the TGUI-Changes Contains changes to TGUI. Make sure its up to date with TGUI 4.0 label Jan 19, 2025
Copy link
Member

@PowerfulBacon PowerfulBacon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a look at a few UIs and the internal code, relatively simple change. Will this fail status checks if another TGUI window gets merged and hasn't been updated to use context properly? It would be nice to have some kind of static time checks to make sure bad UIs don't get merged, but I'm not sure if we can do that easilly with javascript (maybe the regex you use can be a lint?)

@itsmeow
Copy link
Member Author

itsmeow commented Jan 19, 2025

JavaScript is difficult to check like that. It'll just be an announcement. Also, my React PR will remove remaining instances.

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@PowerfulBacon PowerfulBacon added this pull request to the merge queue Jan 23, 2025
Merged via the queue into BeeStation:master with commit 6e55c2b Jan 23, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement TGUI-Changes Contains changes to TGUI. Make sure its up to date with TGUI 4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants