Skip to content

Use lib: es5 to prevent IE 11 breaks#8414

Merged
ecraig12345 merged 1 commit intomicrosoft:masterfrom
ecraig12345:lib
Mar 26, 2019
Merged

Use lib: es5 to prevent IE 11 breaks#8414
ecraig12345 merged 1 commit intomicrosoft:masterfrom
ecraig12345:lib

Conversation

@ecraig12345
Copy link
Copy Markdown
Member

@ecraig12345 ecraig12345 commented Mar 21, 2019

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change

Description of changes

Currently we rely on code reviews to prevent ES6 methods and objects, which aren't supported in IE 11, from slipping into our code. Switching all projects which run in the browser to use "lib": ["es5"] (and possibly other relevant libs) in tsconfig.json will prevent these issues.

Related changes:

  • Replace instances of ES6 things with ES5-compatible things
  • Add "skipLibCheck": true to tsconfig.json for projects which have dependencies whose typings use ES6 types
  • In a couple instances where the usage is safe, add file-level declarations for Set/Map/WeakMap--either for tests (which run in Node not the browser) or one place where it checks for the class's existence before using it
Microsoft Reviewers: Open in CodeFlow

"preserveConstEnums": true,
"lib": ["es2017", "dom"],
"skipLibCheck": true,
"lib": ["es5", "es2015.promise", "dom"],
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@Raghurk I noticed there are several components in dashboard which use Promise. Do you require all consumers to provide a Promise polyfill (for IE 11 support)?

private _contentContainer = React.createRef<HTMLDivElement>();
private _subscribers: Set<Function>;
private _stickies: Set<Sticky>;
private _subscribers: Function[];
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Switched Set to arrays here--more verbose but should hopefully have the same behavior

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.

Awesome catch!!!

@size-auditor
Copy link
Copy Markdown

size-auditor bot commented Mar 21, 2019

Bundle test Size (minified) Diff from master
DatePicker 199.315 kB ExceedsBaseline     49 bytes
Calendar 143.597 kB ExceedsBaseline     49 bytes
DetailsList 208.82 kB BelowBaseline     -137 bytes
ShimmeredDetailsList 219.644 kB BelowBaseline     -137 bytes

ExceedsTolerance  Exceeds Tolerance     ExceedsBaseline  Exceeds Baseline     BelowBaseline  Below Baseline     1 kB = 1000 bytes

Copy link
Copy Markdown
Contributor

@kenotron kenotron left a comment

Choose a reason for hiding this comment

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

Thanks for doing this work!

private _contentContainer = React.createRef<HTMLDivElement>();
private _subscribers: Set<Function>;
private _stickies: Set<Sticky>;
private _subscribers: Function[];
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.

Awesome catch!!!

@kenotron
Copy link
Copy Markdown
Contributor

needs rush change :(

@ecraig12345
Copy link
Copy Markdown
Member Author

Planning to merge this EOD today. If anyone who hasn't reviewed wants to take a look before I merge, please do so now (or leave a comment asking me to wait).

@dzearing
Copy link
Copy Markdown
Member

@ecraig12345 Nice work!!!

@ecraig12345
Copy link
Copy Markdown
Member Author

Waiting to merge until I can implement David's suggestion about declaring IE11-compat versions of Set and Map somewhere. (see #8421 (comment) )

@ecraig12345
Copy link
Copy Markdown
Member Author

Updated the PR to add a typings folder at the root with a custom-global package inside (with partial Set/Map/WeakMap types), then referenced that extra typing in oufr, experiments, and utilities (can be added elsewhere as needed).

So this time I really do intend to merge EOD unless someone objects.

@ecraig12345 ecraig12345 merged commit 91c5719 into microsoft:master Mar 26, 2019
@ecraig12345
Copy link
Copy Markdown
Member Author

Merged. If anyone prefers a different approach to the Set/Map typings, let me know and we can revisit the issue.

@ecraig12345 ecraig12345 deleted the lib branch March 26, 2019 01:45
@msft-github-bot
Copy link
Copy Markdown
Contributor

🎉@uifabric/dashboard@v0.58.0 has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Copy Markdown
Contributor

🎉@uifabric/charting@v0.28.10 has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Copy Markdown
Contributor

🎉@uifabric/react-cards@v0.1.1 has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Copy Markdown
Contributor

🎉office-ui-fabric-react@v6.161.0 has been released which incorporates this pull request.:tada:

Handy links:

@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants