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

Removed all imports from barrel files for components, actions, helpers, hooks and lodash #6509

Merged
merged 14 commits into from
Dec 8, 2024

Conversation

pnicolli
Copy link
Contributor

@pnicolli pnicolli commented Dec 2, 2024

Closes #6507

@pnicolli pnicolli requested a review from a team December 2, 2024 19:55
@pnicolli pnicolli self-assigned this Dec 2, 2024
Copy link

netlify bot commented Dec 2, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 83fb4c4
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/6754e747db677e0008ae5cd2

packages/volto/.eslintrc Outdated Show resolved Hide resolved
@pnicolli
Copy link
Contributor Author

pnicolli commented Dec 2, 2024

@stevepiercy I realized the deprecation docs for this already existed, I added a couple extra files I cleaned up.

@pnicolli
Copy link
Contributor Author

pnicolli commented Dec 2, 2024

FYI: this PR also reduces the main client bundle size by almost 30kb gzipped! Just by removing the imports from barrel files!

Copy link
Collaborator

@stevepiercy stevepiercy 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, just add a couple of periods, then I approve docs and news.

Out of curiosity, did you do all this manually or with a tool that finds barrel imports and replaces them? This is a huge amount of changes!

packages/volto-slate/news/6509.internal Outdated Show resolved Hide resolved
packages/volto/news/6509.internal Outdated Show resolved Hide resolved
@pnicolli
Copy link
Contributor Author

pnicolli commented Dec 3, 2024

Out of curiosity, did you do all this manually or with a tool that finds barrel imports and replaces them? This is a huge amount of changes!

@stevepiercy All manually :) I figured that I would have had a lot of 30 to 60 minutes slots during the conf sprint, the following day, the flights and the long layovers, so this task would have been a good fit

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Docs and news approved. Still needs tech review.

@stevepiercy
Copy link
Collaborator

Needs an ESLint fix, then should be good to merge.

@sneridagh
Copy link
Member

@pnicolli now should be green, let's merge it and release it.
Thanks for taking care, it rocks!

@sneridagh
Copy link
Member

@pnicolli just realised of something:
if we leave the ESlint rules "on" by default, they will break all projects outside, so we cannot enable it by default in 18.
:/ MEH!

Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

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

LGTM and ready to merge.

@pnicolli waiting for your last ack.

@sneridagh sneridagh force-pushed the pnicolli-remove-barrel-files branch from 9c42587 to a118254 Compare December 5, 2024 18:23
* main:
  Release 18.1.2
  Remove devserver settings for HMR of volto (#6511)
  Release @plone/components 2.2.1
  [components] Fix color picker usability (#6512)
  Fix README in npm by copying it from the root of the monorepo. (#6510)
@davisagli
Copy link
Member

@sneridagh @pnicolli Are we merging this for Volto 18 or 19? If it's for 18, it's a bit late (we already made non-alpha releases and this is breaking). If it's for 19, fine, but then we need to cut an 18.x branch and start using main for 19.

@davisagli
Copy link
Member

@sneridagh @pnicolli Oh sorry, now that I actually read the PLIP, I understand that this is fixing the imports in Volto core but not actually removing the barrels yet. Okay!

@pnicolli
Copy link
Contributor Author

pnicolli commented Dec 6, 2024

I found another barrel file that I didn't see in the first iteration, I worked on it last night, will finish asap.
It is @plone/volto-slate/editor/ui

@sneridagh
Copy link
Member

@davisagli yes, we realised that the work of moving all the imports to direct imports is fine and non-breaking. This will also enable right now multi-framework add-ons :)

@pnicolli
Copy link
Contributor Author

pnicolli commented Dec 8, 2024

Ok added the last small commit, now I think it's good to go @sneridagh

@sneridagh sneridagh merged commit dd35efb into main Dec 8, 2024
69 checks passed
@sneridagh sneridagh deleted the pnicolli-remove-barrel-files branch December 8, 2024 08:13
sneridagh added a commit that referenced this pull request Dec 8, 2024
* main:
  Commit missing README from release
  Release 18.2.0
  Release @plone/slate 18.0.2
  Removed all imports from barrel files for components, actions, helpers, hooks and lodash (#6509)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Remove any usage of barrel files from core
4 participants