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

Use eslint for sorting our imports #1637

Merged
merged 6 commits into from
Feb 4, 2022
Merged

Use eslint for sorting our imports #1637

merged 6 commits into from
Feb 4, 2022

Conversation

hgiesel
Copy link
Contributor

@hgiesel hgiesel commented Feb 2, 2022

One overhead I often have while working on the TS code is sorting imports. This would leave it to eslint to sort our imports (with bazel run eslint -- --fix)

In the process I also applied eslint generally to .svelte files, which revealed some issues. I would use this PR, to fix those and fit all requirements. Preferably after #1626 :)

@dae
Copy link
Member

dae commented Feb 3, 2022

Seems reasonable, though it looks like it also triggers an 'exports must be at bottom' warning that needs to be manually addressed (eg in card-info)

@dae
Copy link
Member

dae commented Feb 3, 2022

.. and it looks like a bunch of other issues we weren't aware of need addressing too ^_^;

@hgiesel
Copy link
Contributor Author

hgiesel commented Feb 3, 2022

Yep. I'm still at odds with myself, whether we should actually use the 'exports-last' rule, but most of the other changes seem reasonable.

hgiesel and others added 2 commits February 3, 2022 16:41
Caught on Linux due to the stricter sandboxing
- We use ResizeObserver which is not supported in browsers like KaiOS,
  Baidu or Android UC
- It's the first version that supports ResizeObserver
@@ -89,6 +89,6 @@
"not op_mini all",
"not < 1%",
"Chrome 77",
"iOS 12"
"iOS 13.4"
Copy link
Contributor Author

@hgiesel hgiesel Feb 3, 2022

Choose a reason for hiding this comment

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

That's the minimum version to support ResizeObserver. If the editor makes it AnkiMobile before 13.4 is more spread, we could just generally turn off the image resizing and Mathjax editor for AnkiMobile (that's the two cases were we use ResizeObserver).

Copy link
Member

Choose a reason for hiding this comment

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

While it will probably be a while until I get the editor ported over, a bump to 13.4 is not too far off, and may come in the next update, so this should be fine. 👍

@dae
Copy link
Member

dae commented Feb 4, 2022

Thanks for sorting those out Henrik, it's nice to have the extra linting coverage, and the more the code formatting can be automated, the better.

@dae dae merged commit 30bbbaf into ankitects:main Feb 4, 2022
@hgiesel hgiesel deleted the sortimports branch February 4, 2022 13:46
@hgiesel
Copy link
Contributor Author

hgiesel commented Feb 4, 2022

I broke the editor overlays (Mathjax editor, Image resizer) in this PR 🤷‍♂️ I'm working on it.

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

Successfully merging this pull request may close these issues.

2 participants