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

chore: regenerate dependency graph to reduce module redundancies #4323

Closed
wants to merge 18 commits into from

Conversation

radium-v
Copy link
Collaborator

@radium-v radium-v commented Feb 3, 2021

Description

This PR draft should definitely be split apart before merging. It's just way too big to go in wholesale. I'm opening it to get discussion started on its parts so it can be more easily separated as we move forward with it.

This is a massive change that has very few actual moving parts. The bulk of it can be summarized as:

  • Dedupe redundant packages - namely lodash/lodash-es, babel, and eslint as well as their related plugins and extensions. Aside from monaco-editor, this was the biggest culprit responsible the size explosion in node_modules.
  • Regenerate the yarn.lock file. This is a controversial step, but since so many packages and projects have moved in and out of the repository in the last year, there were a lot of leftover extras that were being too aggressively specified, which lead to a reduction in overall hoisting opportunities.
  • Add explicit types where needed to avoid running into [api-extractor] import() types are not handled correctly in .d.ts rollups rushstack#1050. All *.template.ts and *.styles.ts modules now have explicit types, which prevents tsc from generating declaration files with import() statements for types.
  • Add documentation blocks to all style modules. These exports were lacking doc blocks, so they weren't showing up in the documentation.
  • Fix as many type errors and eslint errors as I could find. Most of the eslint errors were for import sorting and missing types. Most of the type errors were the result of improperly typed parameters and missing types. The biggest issues by far were the placements of any, object, and {} types, which now throww errors when they're used.
  • Replace type-only imports with import type statements. This is a feature that's been around since TypeScript 3.8, and is super nice.
  • Update Storybook stories to use side-effect only imports for modules (for example: import "./something";) and normalize naming for exports. This makes stories much simpler to write, since we don't have to use the modules to avoid tree-shaking.
  • Fix eslint to prevent @typescript-eslint from throwing errors in JavaScript files (which for us are mostly configuration files).

To test this, please clean your local workspace entirely after checking out the branch:

yarn lerna clean
git clean -ndx # dry run, make sure to check the output before running the next command
git clean -dfx # removes all directories and files that aren't tracked by git
yarn

Issue type checklist

  • Chore: A change that does not impact distributed packages.
  • Bug fix: A change that fixes an issue, link to the issue above.
  • New feature: A change that adds functionality.

Is this a breaking change?

  • This change causes current functionality to break.

I checked as much of the project as I could, but I'm not sure what the expected results are for every package in every part of the repository. Fingers crossed that I caught everything 🤞

Adding or modifying component(s) in @microsoft/fast-components checklist

Process & policy checklist

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 17540 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 17537 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Feb 3, 2021

Code Climate has analyzed commit d6e6841 and detected 45 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 44

View more on Code Climate.

@radium-v radium-v closed this Apr 8, 2021
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.

1 participant