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

Enforce path imports for mui icons, Migrate to newer eslint (v8) #572

Merged
merged 4 commits into from
Feb 13, 2025

Conversation

astitv-sh
Copy link
Collaborator

References

Changes to resolve #571. Follows the changes made in similar PR jupyterlab/jupyter-ai#1225

Code changes

  • eslintrc.js updated to include restricted path import checks
  • package.json updated for migration to eslint v8
  • yarn.lock generated with the aforementioned changes

User-facing changes

N/A

Testing

  1. After enforcing path import checks, CI fails with the expected error message (tested by importing incorrectly):

Screenshot 2025-02-10 at 8 32 28 PM

  1. Package bundle size drops on fixing path imports (tested by importing incorrectly):

Before:

Screenshot 2025-02-10 at 1 20 46 PM

After:

Screenshot 2025-02-10 at 1 21 29 PM

Backwards-incompatible changes

N/A

@astitv-sh astitv-sh requested a review from andrii-i February 10, 2025 15:56
@astitv-sh astitv-sh added the enhancement New feature or request label Feb 10, 2025
Copy link
Collaborator

@andrii-i andrii-i left a comment

Choose a reason for hiding this comment

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

Thank you for working on this @astitv-sh.

Summarizing our discussion on next steps:

  • To address three @typescript-eslint/no-unused-vars warnings from the "Build" CI run, please omit catch parameter in all cases by changing catch statements with catch parameters to catch statements without catch parameters:
} catch {
  • To address the lint error, please update the lint rule name in the disable comment
  • I will fix E2E failures

@andrii-i
Copy link
Collaborator

@astitv-sh thank you for the updates. Please investigate the CI failure by looking at CI error text and identifying the root cause, then potentially fixing it if test failure is not related to the logic being tested.

@andrii-i
Copy link
Collaborator

andrii-i commented Feb 13, 2025

1dbc0d6 sets a no-op filter if extraction filter is supported. Extraction filter is supported in Python 3.12+ and throws warnings if not present. It will become required in 3.14+ https://docs.python.org/3.14/library/tarfile.html#supporting-older-python-versions

Copy link
Collaborator

@andrii-i andrii-i left a comment

Choose a reason for hiding this comment

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

Looks good, thank you @astitv-sh. I tested changes with Python 3.11 and Python 3.12.9.

@andrii-i andrii-i merged commit 469b8e6 into jupyter-server:main Feb 13, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enforce path imports for MUI icons, upgrade to ESLint v8
2 participants