Skip to content

fix(scripts-generators): create manifest only from v8 monorepo packages within generate-package-manifest #29579

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.

Sign up for GitHub

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

Jump to bottom
Merged
Hotell merged 1 commit into from
Nov 1, 2023
Merged

fix(scripts-generators): create manifest only from v8 monorepo packages within generate-package-manifest #29579

Hotell merged 1 commit into from
Nov 1, 2023

Conversation

@Hotell
Copy link
Contributor

@Hotell Hotell commented Oct 18, 2023 •
edited
Loading

Previous Behavior

generate manifests adds packages that are not v8 related.

New Behavior

  • generate manifests adds only v8 related packages and makes the contract correct
image
  • script prints paths that have been created for better DX
image
  • script is now generic and can be customized via CLI in future if needed

Related Issue(s)

  • Fixes #

@Hotell Hotell added Area: Build System Fluent UI react (v8) Issues about @fluentui/react (v8) labels Oct 18, 2023
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 18, 2023 •
edited
Loading

📊 Bundle size report

🤖 This report was generated against aa6771ed95c5af78f413b3c20068867f032cf4c4

Hotell
Hotell commented Oct 18, 2023
View reviewed changes
Hotell
Hotell commented Oct 18, 2023
View reviewed changes
@Hotell Hotell force-pushed the hotell/fix-generate-package-manifest branch from c27e1d3 to 301b051 Compare October 18, 2023 14:34
@Hotell Hotell marked this pull request as ready for review October 18, 2023 14:35
@Hotell Hotell requested a review from a team as a code owner October 18, 2023 14:35
@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 18, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 301b051:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Oct 18, 2023

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: aa6771ed95c5af78f413b3c20068867f032cf4c4 (build)

@Hotell Hotell changed the title fix(scripts-generators): refacrtor generate-package-manifest to create manifest only from v8 monorepo packages fix(scripts-generators): create manifest only from v8 monorepo packages within generate-package-manifest Oct 18, 2023
ling1726
ling1726 reviewed Oct 26, 2023
View reviewed changes
scripts/generators/generate-package-manifest.js
function main(tag, manifestRoot, rootPackage) {
const allPackageInfo = getAllPackageInfo();

const filteredPackages = Object.entries(allPackageInfo).reduce((acc, curr) => {
Copy link
Contributor

@ling1726 ling1726 Oct 26, 2023

Choose a reason for hiding this comment

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

nit: might be easier to call Object.fromEntries after filtering by the object entries

Copy link
Contributor Author

@Hotell Hotell Nov 1, 2023

Choose a reason for hiding this comment

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

can you elaborate a bit more ?

You mean using following ?

Object.fromEntries(Object.entries(allPackageInfo).map(...)) ?

if that's the case I dont think its more readable/faster than reduce

ling1726
ling1726 reviewed Oct 26, 2023
View reviewed changes
@Hotell Hotell merged commit d35fc26 into microsoft:master Nov 1, 2023
@Hotell Hotell deleted the hotell/fix-generate-package-manifest branch November 1, 2023 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GeoffCoxMSFT GeoffCoxMSFT GeoffCoxMSFT approved these changes

@marcosmoura marcosmoura Awaiting requested review from marcosmoura

+1 more reviewer

@ling1726 ling1726 ling1726 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area: Build System Fluent UI react (v8) Issues about @fluentui/react (v8)

Projects

None yet

Milestone

October Project Cycle Q4 2023

Development

Successfully merging this pull request may close these issues.

4 participants

@Hotell @fabricteam @ling1726 @GeoffCoxMSFT