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

build improvement: enable explicit-member-accessibility ESLint rule #376

Open
43081j opened this issue Jan 10, 2022 · 2 comments
Open

build improvement: enable explicit-member-accessibility ESLint rule #376

43081j opened this issue Jan 10, 2022 · 2 comments

Comments

@43081j
Copy link
Collaborator

43081j commented Jan 10, 2022

We should set @typescript-eslint/explicit-member-accessibility to error.

This will require that all members now have accessibility modifiers (e.g. public).

Should be an easy one, though it will probably uncover a lot of places we are misusing protected methods as if they were public (where protected ones right now are prefixed with _).

@fb55
Copy link
Collaborator

fb55 commented Mar 7, 2022

The big issue here is the parser, which uses a lot of internal methods, which are used by functions called by the parser (eg. startTagInCell). I am unsure of the best way forward here; one option is to just add all of these functions as methods to the parser.

@43081j
Copy link
Collaborator Author

43081j commented Mar 7, 2022

yup, its just poorly structured right now.

many of the parser's methods are prefixed with _ as if they are protected/private, but that is a lie because countless functions which consume a parser access those methods from outside.

with the current structure i'd just make the methods public that are accessed from those functions, as thats what they really are, whether we like it or not.

longer term i'd prefer seeing those functions become part of the parser like you say. though that'd result in a crazily huge parser class, so thats another problem we should solve too: extracting much of the logic into utilities/helpers/other new classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants