Skip to content

Require Scratch.Cast in ESLint #2029

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

Closed
wants to merge 2 commits into from

Conversation

Brackets-Coder
Copy link
Contributor

This is the first time I've done something like this, and I was just following the patterns already in the file and in #1855, so I hope I did this right.

I don't think this will get merged, I probably did something dumb somewhere and I haven't tested this...

Anyway, I added a rule to eslint.config.js to return an ESLint error if using Cast instead of Scratch.Cast for all future extensions. Don't know if it will work.

Add Scratch.Cast as a requirement
@github-actions github-actions bot added the pr: other Pull requests that neither add new extensions or change existing ones label Mar 11, 2025
@Brackets-Coder
Copy link
Contributor Author

Well, I guess it works, because I forgot to add ESLint ignores for the new rules to all of the old extensions and now they all return errors every time Cast is used instead of Scratch.Cast. Oops, that'll need to be fixed. I don't feel like going through all of the extensions right now so I'll probably get around to the Lint errors later.

This should make new extensions switch to Scratch.Cast for best practices.

@GarboMuffin
Copy link
Member

Aliasing a local copy of Cast is perfectly fine..

@Brackets-Coder
Copy link
Contributor Author

Aliasing a local copy of Cast is perfectly fine..

I'm aware of this, and that's one of the biggest oversights of this PR, but is there a way we can check if a file contains just Cast but not Scratch.Cast (and therefore does not use Cast properly) and return an error? Many extensions use something like
const Cast = Scratch.Cast but if Scratch.Cast isn't present in the entire file but a Cast is then it should return an error.

@GarboMuffin
Copy link
Member

That is already an ESLint error because then Cast is not defined and ESLint should be able to detect this already. It will also be caught during any reasonable testing process because that block would encounter a ReferencError.

@GarboMuffin
Copy link
Member

GarboMuffin commented Mar 11, 2025

In general if we're adding a new ESLint rule there should be an example somewhere of incorrect code that it produces a valid warning for. The change here only finds errors in https://github.com/TurboWarp/extensions/blob/3dc6a5a76550346452beeccfd0df799f2d948156/extensions/Lily/Assets.js and https://github.com/TurboWarp/extensions/blob/3dc6a5a76550346452beeccfd0df799f2d948156/extensions/JeremyGamer13/tween.js both of which have const Cast = Scratch.Cast; which indicates that the problem of using Cast without defining it first doesn't really exist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: other Pull requests that neither add new extensions or change existing ones
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants