noahdarveau/Rollup changes for treeshaking #2513
Conversation
noahdarveau-MSFT
changed the title
noahdarveau-MSFT
changed the title
| TargetFolder: '$(Build.ArtifactStagingDirectory)\NPMFeed\dist' | ||
| flattenFolders: true | ||
| flattenFolders: false |
There was a problem hiding this comment.
I assume this is to for backcompat in case a developer doesn't have ESModules configured
There was a problem hiding this comment.
Yes, that is one reason, but mostly it is because the rollup-built package keeps all of the files separate, so if we flatten all of the files to one folder level, our published artifact would be a huge mess of every file in teams-js condensed to a single folder level. Keeping the folder structure helps keep the artifact neat.
There was a problem hiding this comment.
Got it, thanks for the explanation
| import { barCode } from 'testlibraryfortreeshaking'; | ||
| //import { PrintTestMessage } from 'testlibraryfortreeshaking/app'; | ||
| import { geoLocation } from '@microsoft/teams-js'; | ||
| //import { app } from '@microsoft/teams-js'; |
There was a problem hiding this comment.
Do we want to keep these commented out? If so can we have an added comment explaining why
There was a problem hiding this comment.
Oh, yeah, I forgot about that file. The whole tree-shaking test app isn't actually doing anything. I was just using it for testing, and wanted to check it in for use later. I'll go ahead and remove the comments so it looks nicer.
| @@ -18,7 +18,7 @@ export { | |||
| registerUserSettingsChangeHandler, | |||
| openFilePreview, | |||
| } from './privateAPIs'; | |||
| export { conversations } from './conversations'; | |||
| export { conversations, OpenConversationRequest, ConversationResponse } from './conversations'; | |||
There was a problem hiding this comment.
What is this change needed for?
There was a problem hiding this comment.
While developing, I was running into a bug where OpenConversationRequest and ConversationResponse were not able to be imported by the teams test app, so I had to export it directly to resolve the issue. I just tried removing the change and it seems to be working ok, so I've reverted the change.
For more information about how to contribute to this repo, visit this page.
Description
Main changes in the PR:
mainfield point to the minified cdn bundle and added themodulefield to point to the esm bundle.Validation
Validation performed: