AG-11628 widgets tree shaking #7981
Conversation
| @@ -355,40 +333,6 @@ export class GridCoreCreator { | |||
| return seed; | |||
| } | |||
|
|
|||
| private createAgStackComponentsList(registeredModules: Module[]): any[] { | |||
There was a problem hiding this comment.
No longer required as we register widgets / ag stack components alongside with the templates that they are used.
| @@ -1,9 +1,5 @@ | |||
| import { _getFunctionName } from '../utils/function'; | |||
|
|
|||
| export function QuerySelector(selector?: string): Function { | |||
There was a problem hiding this comment.
Not used anywhere so removed.
| continue; | ||
| } | ||
| // each component must have a unique selector | ||
| let name = comp.selector.toUpperCase(); |
There was a problem hiding this comment.
Upper case is only being applied on the set, it's not used on the has check here, or in the getComponent method.
Why do we need to upper case at all? Can't we keep everything lower case?
There was a problem hiding this comment.
The value returned from const key = element.nodeName; in createComponentFromElement is uppercase so we are matching that.
There was a problem hiding this comment.
Don't we need thehas check to also use uppercase then?
Can we add a comment explaining why one is uppercase and the other isn't?
There was a problem hiding this comment.
Added a comment with link to https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeName
There was a problem hiding this comment.
Just made all the selectors uppercase so no need to do any work at runtime.
| import { RefSelector } from '../../widgets/componentAnnotations'; | ||
| import { AgPromise } from '../../utils/promise'; | ||
| import { _clearElement } from '../../utils/dom'; | ||
| import { LayoutCssClasses, LayoutFeature, LayoutView, UpdateLayoutClassesParams } from "../../styling/layoutFeature"; | ||
|
|
||
| import { OverlayService } from './overlayService'; | ||
|
|
||
| export class OverlayWrapperComponent extends Component implements LayoutView { | ||
| export class AgOverlayWrapper extends Component implements LayoutView { |
There was a problem hiding this comment.
File name doesn't match the class name now
There was a problem hiding this comment.
Ah I reverted a lot of these but missed this. Will put back to its current name as I was aiming to reduce noise in the PR.
| @@ -224,31 +246,23 @@ export class Component extends BeanStub { | |||
| elements.forEach(el => el.setAttribute('tabindex', tabIndex.toString())); | |||
| } | |||
|
|
|||
| public setTemplate(template: string | null | undefined, paramsMap?: { [key: string]: any; }): void { | |||
| public setTemplate(template: string | null | undefined, components: ComponentClass[], paramsMap?: { [key: string]: any; }): void { | |||
There was a problem hiding this comment.
components should be optional
There was a problem hiding this comment.
made components optional and removed umpty arrays.
| // if(!current){ | ||
| // (this as any)[elementRef] = newComponent ?? element; | ||
| // } | ||
| // } |
There was a problem hiding this comment.
Should this still be here?
|
|
||
| export interface AgInputTextFieldParams extends AgInputFieldParams { | ||
| allowedCharPattern?: string; | ||
| } | ||
|
|
||
| export class AgInputTextField<TConfig extends AgInputTextFieldParams = AgInputTextFieldParams> extends AgAbstractInputField<HTMLInputElement, string, TConfig> { | ||
| static readonly selector: AgComponentSelector = 'ag-input-text-field'; |
There was a problem hiding this comment.
This is extended by other widgets, which have different selectors, e.g AgInputNumberField and AgInputDateField
There was a problem hiding this comment.
Have added the selectors to the extending classes
| 'ag-watermark' | | ||
| 'ag-fill-handle' | | ||
| 'ag-range-handle' | | ||
| 'ag-color-picker' | |
There was a problem hiding this comment.
I don't like that we're referring to module widgets in core
There was a problem hiding this comment.
This is just for the typing. This means that we now get type checking in React where it looks up components via their tags in gridComp.tsx
We already have other interfaces in core that contain enterprise level features.
The main goal of these changes to remove the static registration of all widgets in the grid.ts file. Instead each component provides the widgets that are required for its own template. This is a similar pattern to Angular's standalone components.
I have also made the selector name explicit in each of the component and that name is used to match against the tags in templates. So for example you will now find the string
ag-grid-bodyon theGridBodyCompclass. This has also enabled us to strongly type the complete list of component tags.This improves type safety when in React we see if components are registered to check for the presence of feature modules.
Each feature module automatically registers all its own components as the assumption is that they will all be required. This is also required to make the React optional registration work.
A number of enterprise only widgets have now been moved to
ag-grid-enterprise/coreand imported from there if used in multiple enterprise features.The initial result of this work is a 7% drop in bundle size. However, as Editing and Filtering are still included by default those widgets are maintained even if filtering and editing is not enabled.