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

Syntax fixes #5367

Merged
merged 2 commits into from
Apr 29, 2021
Merged

Syntax fixes #5367

merged 2 commits into from
Apr 29, 2021

Conversation

paescuj
Copy link
Member

@paescuj paescuj commented Apr 29, 2021

As discussed in #5254 this pull request contains only a slimmed down version of the "syntax fixes" part.

There are still many changes, but it is much clearer now and the changes should be safer and easier to be reviewed.
It is now split into two commits:

  • One which declares return types on TypeScript functions (65032f2)
  • One which contains minor fixes, mostly based on warnings from eslint (see b70de1b for detailed explanation)

Many changes from the original pull request have been omitted:

  • All the changes which were done automatically by eslint or stylelint, for obvious reasons
  • All the changes which are "easily" reproducible, like:
    • Changes related to hasOwnProperty: @rijkvanzanten IMHO I still think those should be changed to Object.prototype.hasOwnProperty.call or otherwise the corresponding eslint rule should be disabled - I don't see any advantage in changing it into 'key' in transformation and didn't want to waste my time on this
    • Removal of unused imports, variables and types
  • I left out almost all changes to non-null assertions because those vary from case to case and cannot easily be changed (as @rijkvanzanten already stated)

I've checked all the changes again to make sure everything is alright and rebased it on latest state from main branch.

I really hope you can use this pull request as I've now put quite a lot of effort into it 😃 Thanks!

paescuj added 2 commits April 29, 2021 15:09
And a very few other type related minor fixes
* Remove unnecessary escape chars in regexes
* Remove unnecessary awaits
* Replace deprecated req.connection with req.socket
* Replace deprecated upload with uploadOne
* Remove unnecessary eslint-disable-next-line comments
* Comment empty functions / catch or finally clauses
* Fix irregular whitespaces
* Add missing returns (null)
* Remove unreachable code
* A few logical fixes
* Remove / Handle non-null assertions which are certainly unnecessary (e.g. in
tests)
@paescuj
Copy link
Member Author

paescuj commented Apr 29, 2021

Just as an information: With this pull request the ESLint problems have been reduced from around 750 (state from #5312 (comment)) to 360, where 175 will be automatically fixable:

✖ 360 problems (230 errors, 130 warnings)
  175 errors and 0 warnings potentially fixable with the `--fix` option.

@rijkvanzanten rijkvanzanten merged commit acd41eb into directus:main Apr 29, 2021
AustinPhillipTaylor pushed a commit to AustinPhillipTaylor/directus that referenced this pull request May 11, 2022
* Declare return types on functions

And a very few other type related minor fixes

* Minor syntax fixes

* Remove unnecessary escape chars in regexes
* Remove unnecessary awaits
* Replace deprecated req.connection with req.socket
* Replace deprecated upload with uploadOne
* Remove unnecessary eslint-disable-next-line comments
* Comment empty functions / catch or finally clauses
* Fix irregular whitespaces
* Add missing returns (null)
* Remove unreachable code
* A few logical fixes
* Remove / Handle non-null assertions which are certainly unnecessary (e.g. in
tests)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants