You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently bundlers like rollup, webpack, and parcel are unable to analyze the mgt packages and determine which parts of the these dependencies are live code to be included. This is often called "Tree Shaking", dead code removal. To enable this behavior our libraries must be side effect free, or refactor the side effects in such a way that they can be avoided by developers.
The current implementation of our @microsoft/mgt-components relies on the side effects of using the @customElement decorator to register the custom elements with the browser as part of the import of any dependency from the library at the top level, i.e. import { xyz } from '@microsoft/mgt-components';. This results in all of the custom elements being registered as soon as one top level import exists. Completely removing this side effect would be a breaking change and one that we want to avoid until we're ready for a v4 release.
Given these constraints it is necessary to:
Eliminate the usage of a decorator to provide custom element registraton
Open question, should we create a custom eslint rule to scan for usages of the @customElement decorator and report these as errors?
Provide a function for registering a given custom element which calls the registration functions for each of the custom elements that the component directly relies upon. Using a registration function is consistent with the way in which @fluentui/web-component approaches component registration.
Ensure that importing from the top level of the @microsoft/mgt-components library still has a side effect of registering all custom elements.
Provide a simple second level import point that is free of side effects for @microsoft/mgt-components, e.g. import { MgtPersonCard } from '@microsoft/mgt-components/exports;
Refactor the code generation of the @microsoft/mgt-react library such that it avoids the top level side effect in @microsoft/mgt-components and allows correct tree shaking by bundlers.
Ensure that any use of the disambiguation tooling still results in registration of correctly disambiguated tags with the browser
Description of the changes
Marks most libraries as being side effect free in package.json to allow bundlers to determine which code should be included.
Marks the root index file in mgt-components as having side effects.
Removes the use of the @customElement decorator as this is a side effect generating pattern that adds code which executes at import time to register the custom elements.
Creates a new register{{ComponentClass}}Component() function per component to provide an imperative function based component mechanism for registering a custom element, including any nested custom element usages. Having a registration function that uses a naming convention is essential to allow the React shims to be generated based on metadata generated via custom-elements-mainifest/analyzer.
Refactors React shim generation to be file per shim component and a barrel file, this ensures that bunders can include dependencies via React usage on a per-file basis and eliminates ambiguity as to which dependencies are required for a given React component.
Refactors a number of functions/classes/interfaces into separate files to reduce the number of circular dependencies down to two chains (mgt-person => mgt-person-card => mgt-person & mgt-person => mgt-person-card => mgt-organization => mgt-person)
Adds a plugin for custom-elements-mainifest/analyzer to correctly determine out custom element definitions at build time for use in generation of the react package and metadata used in storybook for docs pages, this is necessary as our components now use a custom function to perform the registration of the custom element which the custom-elements-mainifest/analyzer is unable to detect and understand natively.
Refactored the import of MgtPersonCard in MgtPerson to become a dynamic import.
PR checklist
Project builds (yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)
All public APIs (classes, methods, etc) have been documented following the jsdoc syntax
Stories have been added and existing stories have been tested
License header has been added to all new source files (yarn setLicense)
Contains NO breaking changes
Other information
Still to do, try to sort out dynamic imports in mgt-person so that we only pull in the mgt-person-card when needed.
BEGIN_COMMIT_OVERRIDE
feat!: enable live code inclusion for consuming applications (#2642)
Packages are marked as side effect free or have the files that include side effects listed in package.json. This allows bundlers to discard code not in the direct dependency tree of an import.
Adds functions for registering components
Moved to generating a single file per react wrpper component and a barrel file
Update tests to use registerComponent calls
fixing render problem when given and surname are null on personType objects
move mgt-components to module type es2020 for dynamic imports
refactored to only lazy load the person-card code when opening the flyout
update list of components registered by registerMgtComponents
move sample app to use lazy + Suspense to chunk up component usage
adding documentation for tree shaking support
BREAKING CHANGE: applications importing @microsoft/mgt-react but not using the wrapper components will not have components automatically registered in the browser. This leads to a breaking change when these applications emit raw web component markup rather than using the wrapper components.
END_COMMIT_OVERRIDE
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gavinbarron
dismissed stale reviews from musale and sebastienlevert
via
Closes #1457
Closes #2656
PR Type
Design considerations
Currently bundlers like rollup, webpack, and parcel are unable to analyze the mgt packages and determine which parts of the these dependencies are live code to be included. This is often called "Tree Shaking", dead code removal. To enable this behavior our libraries must be side effect free, or refactor the side effects in such a way that they can be avoided by developers.
The current implementation of our
@microsoft/mgt-componentsrelies on the side effects of using the@customElementdecorator to register the custom elements with the browser as part of the import of any dependency from the library at the top level, i.e.import { xyz } from '@microsoft/mgt-components';. This results in all of the custom elements being registered as soon as one top level import exists. Completely removing this side effect would be a breaking change and one that we want to avoid until we're ready for a v4 release.Given these constraints it is necessary to:
@customElementdecorator and report these as errors?@fluentui/web-componentapproaches component registration.@microsoft/mgt-componentslibrary still has a side effect of registering all custom elements.@microsoft/mgt-components, e.g.import { MgtPersonCard } from '@microsoft/mgt-components/exports;@microsoft/mgt-reactlibrary such that it avoids the top level side effect in@microsoft/mgt-componentsand allows correct tree shaking by bundlers.Description of the changes
Marks most libraries as being side effect free in package.json to allow bundlers to determine which code should be included.
Marks the root index file in mgt-components as having side effects.
Removes the use of the
@customElementdecorator as this is a side effect generating pattern that adds code which executes at import time to register the custom elements.Creates a new
register{{ComponentClass}}Component()function per component to provide an imperative function based component mechanism for registering a custom element, including any nested custom element usages. Having a registration function that uses a naming convention is essential to allow the React shims to be generated based on metadata generated via custom-elements-mainifest/analyzer.Refactors React shim generation to be file per shim component and a barrel file, this ensures that bunders can include dependencies via React usage on a per-file basis and eliminates ambiguity as to which dependencies are required for a given React component.
Refactors a number of functions/classes/interfaces into separate files to reduce the number of circular dependencies down to two chains (mgt-person => mgt-person-card => mgt-person & mgt-person => mgt-person-card => mgt-organization => mgt-person)
Adds a plugin for custom-elements-mainifest/analyzer to correctly determine out custom element definitions at build time for use in generation of the react package and metadata used in storybook for docs pages, this is necessary as our components now use a custom function to perform the registration of the custom element which the custom-elements-mainifest/analyzer is unable to detect and understand natively.
Refactored the import of MgtPersonCard in MgtPerson to become a dynamic import.
PR checklist
yarn build) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)yarn setLicense)Other information
Still to do, try to sort out dynamic imports in mgt-person so that we only pull in the mgt-person-card when needed.
BEGIN_COMMIT_OVERRIDE
feat!: enable live code inclusion for consuming applications (#2642)
Packages are marked as side effect free or have the files that include side effects listed in package.json. This allows bundlers to discard code not in the direct dependency tree of an import.
Adds functions for registering components
Moved to generating a single file per react wrpper component and a barrel file
Update tests to use registerComponent calls
fixing render problem when given and surname are null on personType objects
move mgt-components to module type es2020 for dynamic imports
refactored to only lazy load the person-card code when opening the flyout
update list of components registered by registerMgtComponents
move sample app to use lazy + Suspense to chunk up component usage
adding documentation for tree shaking support
BREAKING CHANGE: applications importing
@microsoft/mgt-reactbut not using the wrapper components will not have components automatically registered in the browser. This leads to a breaking change when these applications emit raw web component markup rather than using the wrapper components.END_COMMIT_OVERRIDE