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, upgrade to ESLint v8 #1225

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Feb 4, 2025

  • Bumps eslint to v8 required to add enforcement rules for path imports on mui

  • Enforces new icons - the CI correctly fails on first commit:

     /home/runner/work/jupyter-ai/jupyter-ai/packages/jupyter-ai/src/components/chat-settings.tsx
       223:16  warning  'e' is defined but never used  @typescript-eslint/no-unused-vars
     
     /home/runner/work/jupyter-ai/jupyter-ai/packages/jupyter-ai/src/components/settings/existing-api-keys.tsx
       9:1  error  '@mui/icons-material' import is restricted from being used. Please import icons using path imports, e.g. `import AddIcon from '@mui/icons-material/Add'`  no-restricted-imports
     
     /home/runner/work/jupyter-ai/jupyter-ai/packages/jupyter-ai/src/components/settings/model-fields.tsx
       29:18  warning  'exc' is defined but never used  @typescript-eslint/no-unused-vars
     
     /home/runner/work/jupyter-ai/jupyter-ai/packages/jupyter-ai/src/handler.ts
       34:14  warning  'error' is defined but never used  @typescript-eslint/no-unused-vars
     
     /home/runner/work/jupyter-ai/jupyter-ai/packages/jupyter-ai/src/index.ts
       63:14  warning  'e' is defined but never used  @typescript-eslint/no-unused-vars
     
     /home/runner/work/jupyter-ai/jupyter-ai/packages/jupyter-ai/src/slash-autocompletion.tsx
       5:1  error  '@mui/icons-material' import is restricted from being used. Please import icons using path imports, e.g. `import AddIcon from '@mui/icons-material/Add'`  no-restricted-imports
     
     ✖ 6 problems (2 errors, 4 warnings)
    
  • Fixes jupyter-ai slows down initial page load significantly by downloading 4MB of icons it does not need #1221 - the size of the wheel should drop

Before After
image image
Before After
image image

@krassowski krassowski added the bug Something isn't working label Feb 4, 2025
@krassowski krassowski marked this pull request as ready for review February 4, 2025 12:14
Copy link
Member Author

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

Adding some explanations to help with review

packages/jupyter-ai/.eslintrc.js Show resolved Hide resolved
packages/jupyter-ai/.eslintrc.js Show resolved Hide resolved
@krassowski
Copy link
Member Author

There is a few more places where we should add these eslint rules:

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

@krassowski Awesome work Mike! Thanks for diving deep into this and fixing the issue for Jupyter AI so quickly. You're a legend. 🎉

I'll reach out to @brichet and @andrii-i to let them know about your work here. They can help migrate this change over to their repos. I'll ask them to credit & co-author you on the PRs. 👍

@dlqqq dlqqq changed the title Enforce path imports for mui icons, migrate to newer eslint Enforce path imports for MUI icons, upgrade to ESLint v8 Feb 4, 2025
@dlqqq dlqqq merged commit 5b99b0f into jupyterlab:main Feb 4, 2025
14 checks passed
@dlqqq
Copy link
Member

dlqqq commented Feb 4, 2025

@meeseeksdev please backport to 2.x

Copy link

lumberbot-app bot commented Feb 4, 2025

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 2.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 5b99b0fea04db63c01aa4670ab3ad223373cb9bf
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #1225: Enforce path imports for MUI icons, upgrade to ESLint v8'
  1. Push to a named branch:
git push YOURFORK 2.x:auto-backport-of-pr-1225-on-2.x
  1. Create a PR against branch 2.x, I would have named this PR:

"Backport PR #1225 on branch 2.x (Enforce path imports for MUI icons, upgrade to ESLint v8)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

@ellisonbg
Copy link
Contributor

Thanks Mike, this is great!

dlqqq pushed a commit to dlqqq/jupyter-ai that referenced this pull request Feb 4, 2025
…ade to ESLint v8

* Enforce path imports for mui icons, migrate to newer eslint

* Replace mui icons imports with path imports

(cherry picked from commit 5b99b0f)
dlqqq added a commit that referenced this pull request Feb 4, 2025
…upgrade to ESLint v8) (#1226)

* Backport PR #1225: Enforce path imports for MUI icons, upgrade to ESLint v8

* Enforce path imports for mui icons, migrate to newer eslint

* Replace mui icons imports with path imports

(cherry picked from commit 5b99b0f)

* fix 2.x MUI icon imports

* run yarn dedupe

---------

Co-authored-by: Michał Krassowski <[email protected]>
@krassowski krassowski deleted the enforce-icons branch February 4, 2025 19:56
@brichet
Copy link
Contributor

brichet commented Feb 4, 2025

Thanks @krassowski and @dlqqq for the ping, I'll see how to port it to jupyter-chat 👍

@krassowski
Copy link
Member Author

Thanks for help with the backport! Looking forward to seeing a new 2.x version released :)

@dlqqq
Copy link
Member

dlqqq commented Feb 5, 2025

@krassowski Yes, we were planning on doing a release either today or tomorrow. Looking at the commit history, it seems like a patch release (v2.29.1) would suffice, since I think all of the changes are doc updates & bug fixes. Any objections?

@krassowski
Copy link
Member Author

Sounds good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jupyter-ai slows down initial page load significantly by downloading 4MB of icons it does not need
4 participants