Skip to content
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.

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

feat: create @ui5/webcomponents-ie11 package #2686

Merged
merged 21 commits into from
Jan 20, 2021
Merged

Conversation

vladitasev
Copy link
Contributor

@vladitasev vladitasev commented Jan 14, 2021

Move IE11-specific code from base to a new package

This change reduces the ES6 bundle size by 4KB (by 2KB gzipped) by moving away IE11-specific code. The size of the ES5 bundle remains unchanged.

Changes:

  • @ui5/webcomponents-ie11: new package that maintains all IE-related code. It uses framework hooks to plug seamlessly into the framework.
  • The base package is completely free of IE11 code. It exposes a few new lifecycle hooks.
  • The main and fiori packages depend on @ui5/webcomponents-ie11 and import its features (IE11.js).
  • The Edge.js file, aimed at legacy Edge, has been removed. Legacy Edge should be treated as IE in terms of ES6 support, therefore it will work with the ES5 code, but not with the ES6 code, due to Legacy Edge not supporting object spread operator and rest parameters syntax, which are used throughout the framework and components.
  • Several components have had their IE11 CSS moved to different files and loaded only if IE11 is the current browser.

Refactoring:

  • the base project no longer understands the business logic of IE11 support (such as ShadyDOM, loading the polyfill, the CSS ponyfill, creating the IE style tags) but exposes lifecycle hooks instead.
  • The CSS handling logic has been streamlined for UI5Element.js and StaticAreaItem.js: now they both use the same internal functions and follow the same flow.
  • The getEffectiveStyle function now works for the static area too (has an extra parameter). In addition, this function is now reused by the IE11 code.

IE11 code lifecycle

image

IE11 CSS processing

image

BREAKING CHANGES:

  1. The features, aimed at legacy browser support, are now moved from the base package to the new ie11 package. If you used them before:
  • Add @ui5/webcomponents-ie11 as a dependency to your project

npm i @ui5/webcomponents-ie11 --save
or
yarn add @ui5/webcomponents-ie11

  • Change the following public imports:
    import "@ui5/webcomponents-base/dist/features/browsersupport/IE11.js";
    to
    import "@ui5/webcomponents-ie11/dist/features/IE11.js";
    and
    import "@ui5/webcomponents-base/dist/features/browsersupport/IE11WithWebComponentsPolyfill.js";
    to
    import "@ui5/webcomponents-ie11/dist/features/IE11WithWebComponentsPolyfill.js";
  1. The Legacy Edge browser is no longer supported with the ES6 bundle, therefore the following import has been removed:
    import "@ui5/webcomponents-base/dist/features/browsersupport/Edge.js";
    Legacy Edge is still supported with the ES5 bundle. If your app needs to support Legacy Edge, treat it as IE11 and transpile the code to ES5 as you would for IE11.

@vladitasev vladitasev changed the title feat: create @ui5/webcomponents-compatibility package feat: create @ui5/webcomponents-ie11package Jan 18, 2021
@ilhan007 ilhan007 changed the title feat: create @ui5/webcomponents-ie11package feat: create @ui5/webcomponents-ie11 package Jan 18, 2021
Copy link
Contributor

@pskelin pskelin left a comment

Choose a reason for hiding this comment

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

looks good, just slight naming and documentation remarks.

Also, the PR description says there are lifecycle hooks, but they are events.

@@ -55,6 +54,17 @@ class EventProvider {
});
}

/**
* Fires an event, awaits for all listeners' results to have resolved, and returns the results as an array.
Copy link
Contributor

Choose a reason for hiding this comment

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

technically it doesn't await but returns the promises, could you change the description simply that it returns a promise and is always async?

*
* @param eventName the event to fire
* @param data optional data to pass to each event listener
* @returns {Array} an array with the results of all event listeners
Copy link
Contributor

Choose a reason for hiding this comment

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

a single promise is returned, when resolved it will have the values


// FF, Safari
if (!document.adoptedStyleSheets && !window.ShadyDOM) {
} else if (hasNativeSupport()) { // FF, Safari
Copy link
Contributor

Choose a reason for hiding this comment

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

hasNativeSupport for what? I had to check the implementation, this name is too generic.

@vladitasev vladitasev merged commit 1d3b37e into master Jan 20, 2021
@vladitasev vladitasev deleted the compat-package branch January 20, 2021 12:24
kineticjs pushed a commit to kineticjs/ui5-webcomponents that referenced this pull request Jan 25, 2021
alexandar-mitsev pushed a commit to alexandar-mitsev/ui5-webcomponents that referenced this pull request Feb 1, 2021
NHristov-sap pushed a commit to NHristov-sap/ui5-webcomponents that referenced this pull request Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants