Skip to content

Change all export * in package/framework to named exports#12424

Merged
ghoshkaj merged 6 commits into
microsoft:mainfrom
ghoshkaj:named-export-package-framework
Oct 14, 2022
Merged

Change all export * in package/framework to named exports#12424
ghoshkaj merged 6 commits into
microsoft:mainfrom
ghoshkaj:named-export-package-framework

Conversation

@ghoshkaj
Copy link
Copy Markdown
Contributor

@ghoshkaj ghoshkaj commented Oct 12, 2022

This PR converts export * in package/framework to named exports.

Related issue: #10062

See for the fact that bundle size does not change when the entire repo is converted: #12321 (comment). I could not merge that PR because it is too large for one change and would make main-next integration a nightmare.

@ghoshkaj ghoshkaj requested a review from a team as a code owner October 12, 2022 15:05
@github-actions github-actions Bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct base: main PRs targeted against main branch labels Oct 12, 2022
@ghoshkaj ghoshkaj self-assigned this Oct 12, 2022
@ghoshkaj ghoshkaj requested a review from a team as a code owner October 12, 2022 15:58
@github-actions github-actions Bot added the public api change Changes to a public API label Oct 12, 2022

export { AttachState }

export { CompressedSerializedInterval }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we really need an export per type? can't they all be bundled into a single export statement?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ahh. this is the mark down. blah.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

however, this file is insane. fyi @tylerbutler @Josmithr who i think know about docs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we might want to consider keeping export * for this package, as they are all just re-exports.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's just that this what gets generated by the api-extractor after I run "npm run build" after my script makes the changes to the ts files. I never directly touch the .md files!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I keep the export * in this file, the CI fails to build it.

@github-actions github-actions Bot removed the public api change Changes to a public API label Oct 14, 2022
@ghoshkaj ghoshkaj force-pushed the named-export-package-framework branch from 20c6a26 to 09f8abe Compare October 14, 2022 15:26
@ghoshkaj ghoshkaj force-pushed the named-export-package-framework branch from 09f8abe to 93ad7bc Compare October 14, 2022 16:02
@github-actions github-actions Bot added the public api change Changes to a public API label Oct 14, 2022
@ghoshkaj ghoshkaj merged commit ca9bbf9 into microsoft:main Oct 14, 2022
@ghoshkaj ghoshkaj deleted the named-export-package-framework branch October 14, 2022 19:01
sharptrip pushed a commit to sharptrip/FluidFramework that referenced this pull request Oct 17, 2022
…oft#12424)

This PR converts `export *`  in  `package/framework`  to named exports.
Related issue: microsoft#10062
See for the fact that bundle size does not change when the entire repo is converted:
microsoft#12321 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct base: main PRs targeted against main branch public api change Changes to a public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants