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

Change TS output to ES6 for all libraries #1232

Merged
merged 10 commits into from
Oct 22, 2019
Merged

Change TS output to ES6 for all libraries #1232

merged 10 commits into from
Oct 22, 2019

Conversation

jspruance
Copy link

@jspruance jspruance commented Sep 30, 2019

Relates to issue:
#1076

'botbuilder-core' TS compiler modified to output 'ES5' instead of 'ESNext':

"target": "ES5",
"lib": [ "es2015", "dom" ]

Unit tests pass and browser bot runs with no errors in Chrome (JS errors in IE11 before and after this change).

Note that this change does not guarantee 100% ES5 compatibility since TypeScript compilation only targets syntax and does not do any polyfilling (in contrast, Babel does both). However this should not introduce any new errors - if anything ESNext --> ES5 should be safer.

We could introduce a Babel layer to achieve complete ES5 compatibility, which would look like:
TS > ES6 > Babel > ES5

@coveralls
Copy link

coveralls commented Sep 30, 2019

Pull Request Test Coverage Report for Build 84669

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 87.651%

Totals Coverage Status
Change from base Build 84487: 0.009%
Covered Lines: 4910
Relevant Lines: 5389

💛 - Coveralls

@stevengum
Copy link
Member

@jspruance
Copy link
Author

@jspruance can you add this change to your PR?

https://github.com/microsoft/botbuilder-js/pull/1102/files#diff-dc3dd8e5393908b2139ed0572ce6c0dfR9

@stevengum I've added your changes to the PR

@cleemullins
Copy link
Contributor

Could this be ES6 instead? We have ES6 features (such as in the RegEx). The browsers we target all (likley) support ES6.

@cleemullins
Copy link
Contributor

Conerns are (specifically) that runtime features such as the regex look-ahead, which is used in the recognizers-text, need ES6 features. These features cannot be polyfilled.

Jonathan Spruance (Insight Global) added 2 commits October 15, 2019 15:05
@jspruance
Copy link
Author

This has been updated to es6. Tested in browser echo bot and unit tests passed.

@stevengum stevengum changed the title Change TS output to ES5 for botbuilder-core module Change TS output to ES6 for botbuilder-core module Oct 21, 2019
Copy link
Member

@stevengum stevengum left a comment

Choose a reason for hiding this comment

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

Can you apply the output changes to the other libraries?

@stevengum stevengum changed the title Change TS output to ES6 for botbuilder-core module Change TS output to ES6 for all libraries Oct 21, 2019
@stevengum stevengum added the 4.6 4.6 release label Oct 21, 2019
@jspruance
Copy link
Author

Can you apply the output changes to the other libraries?

This is complete - ready for review.

@Stevenic Stevenic merged commit ad7a319 into master Oct 22, 2019
@Stevenic Stevenic deleted the ts-compile-to-es5 branch October 22, 2019 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.6 4.6 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants