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.

Sign up for GitHub

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

Jump to bottom

Change msal-angular to output es5, fixing compatibility with IE11 #868

Merged
merged 3 commits into from
Sep 10, 2019
Merged

Change msal-angular to output es5, fixing compatibility with IE11 #868

merged 3 commits into from
Sep 10, 2019

Conversation

jasonnutter
Copy link
Contributor

@jasonnutter jasonnutter commented Jul 25, 2019 •
edited
Loading

Fixes MSAL Angular compatibility with IE11 by changing the output to es5.

Key changes:

  • Changed the compilation configuration to ouput es5/commonjs modules instead of es6/es2015 modules, which was not compatible with IE11. The diff is a little confusing, as I renamed the old tsconfig-aot.json to tsconfig.json, and also changed the options accordingly.
  • Removed the webpack scripts and config, as they were not being used (and were a source of confusion), and updated the build scripts to use the ngc command.
  • Removed the dist folder from the repo.
  • Update msal-angular to use msal core 0.2.2 which introduced the storeAuthStateInCookie config flag for IE11.

Once approved, I plan to release as a beta to ensure we haven't introduced any regression like we did last time.

@jasonnutter jasonnutter added msal-angular Related to @azure/msal-angular package compatibility Related to compatibility with specific frameworks, environments, browsers, etc. labels Jul 25, 2019
@DarylThayil
Copy link
Contributor

will this effect any who are using es6 modules / any other side effects?

@jasonnutter
Copy link
Contributor Author

I suppose it might impact es modules in theory, but I'm not sure if anyone uses es modules with Angular in a way that would break (or if that's even possible?).

@DarylThayil
Copy link
Contributor

looks like tests are failing

sameerag
sameerag reviewed Jul 25, 2019
View reviewed changes
Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

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

Should we also push the dist files or add them to gitignore like core and remove them now?

lib/msal-angular/package.json Show resolved Hide resolved
lib/msal-angular/src/msal-guard.service.ts Show resolved Hide resolved
lib/msal-angular/src/msal-guard.service.ts Outdated Show resolved Hide resolved
lib/msal-angular/tsconfig-aot.json Outdated Show resolved Hide resolved
@jasonnutter
Copy link
Contributor Author

@sameerag Yeah we can probably remove the dist files from the repo.

@jasonnutter jasonnutter added this to the 1.2.0 milestone Jul 25, 2019
@jasonnutter jasonnutter force-pushed the angular-es5-ie11 branch 4 times, most recently from 7d50f19 to cc29159 Compare July 26, 2019 21:20
@jasonnutter jasonnutter force-pushed the angular-es5-ie11 branch 2 times, most recently from 79063f6 to 13d1c5a Compare August 1, 2019 23:25
@jasonnutter jasonnutter marked this pull request as ready for review August 1, 2019 23:33
@jasonnutter jasonnutter force-pushed the angular-es5-ie11 branch 2 times, most recently from 5aeeaf8 to 728c3ae Compare August 7, 2019 20:50
@jasonnutter
Merge branch 'dev' of github.com:AzureAD/microsoft-authentication-lib…
2070833 
…rary-for-js into angular-es5-ie11

# Conflicts:
#	.gitignore
#	lib/msal-angular/src/msal-guard.service.ts
@jasonnutter
Update angular changelog, readme, package.json for release
1454879
DarylThayil
DarylThayil approved these changes Sep 9, 2019
View reviewed changes
Copy link
Contributor

@DarylThayil DarylThayil left a comment

Choose a reason for hiding this comment

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

You might be winning the +/- battle on code here

sameerag
sameerag approved these changes Sep 10, 2019
View reviewed changes
Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

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

lgtm. One comment but not a blocker.

@jasonnutter jasonnutter merged commit 3166892 into dev Sep 10, 2019
@sameerag sameerag deleted the angular-es5-ie11 branch November 10, 2020 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DarylThayil DarylThayil DarylThayil approved these changes

@sameerag sameerag sameerag approved these changes

@pkanher617 pkanher617 Awaiting requested review from pkanher617

Assignees

@jasonnutter jasonnutter

Labels
compatibility Related to compatibility with specific frameworks, environments, browsers, etc. msal-angular Related to @azure/msal-angular package
Projects
None yet
Milestone
September 2019
Development

Successfully merging this pull request may close these issues.
3 participants
@jasonnutter @DarylThayil @sameerag