[x] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:
Library version: 1.0.1
Using the es modules directly in a browser throws an error in the browser console trying to load base64.js. This issue was introduced in #712. base64.js does not provide an es module export and a browser can not load the file. See https://github.com/microsoftgraph/microsoft-graph-toolkit/issues/103 for more details.
All es modules in the project should work directly in the browser.
Follow the steps in https://github.com/microsoftgraph/microsoft-graph-toolkit/issues/103 to repro
@nmetulev Thanks for raising this. Will adding {"babel-preset-env": "^1.7.0"} in devdependecies solve this issue? We kind of debated within the team keeping it vs removing it and ultimately decided to not go ahead to keep the dependencies lean.
Let me know and maybe I can push for that.
@DarylThayil FYI - the ES6 dependency I was talking about when we pulled in this change. Looks like it is affecting some customers. Can you take a look too and see if there is another approach apart from what I suggested above?
Thanks for the response @sameerag. I don't believe "babel-preset-env" will solve this since you are generating the es modules directly through tsc and it's not passing through babel
Looks like we need a base64 implementation that is compatible with es modules, correct @nmetulev? Taking a look to see if I can find any that would match that.
Also, I think it would be good to add a sample/test case to our repo to catch Graph Toolkit's use case.
@jasonnutter So you also need to take into account that we need our sec team to approve of any external encoding/decoding library we use. base64.js is approved, we may need to go through all that process if we are deciding to pick a new library.
@sameerag Gotya. Another (potentially simpler) option would be to contribute to base64.js to add the es modules compatibility.
That sounds good. But I believe they have intentionally kept the dependencies minimum to make it a standalone library. We can try!
@nmetulev thanks for letting us know, I agree with @jasonnutter we need a sample so we catch some of this. assigning this to @jasonnutter to follow up on
That sounds good. But I believe they have intentionally kept the dependencies minimum to make it a standalone library. We can try!
They do call out an ES6+ example on their homepage, so it seems like a scenario they want to support? Might just be an oversight on their part for this type of embedded dependency scenario.
Yeah I think unfortunately that es6+ in most libs means, If you are transpiling, this is how you can write your code! vs modules written in es6 to be used directly in the browser.
@DarylThayil, any update on this?
@jasonnutter is currently working on this
@nmetulev We have a potential fix for this that should be included in the next release.
Question: going forward, does this mean that any (non-dev) dependency that we want to use in our library has to be compatible with ES modules (otherwise it would break GTL)?
Thanks @jasonnutter.
Yeah, that makes sense to me. For majority of developers today who use a bundler like webpack or rollup, this shouldn't be an issue, but with es6 modules becoming more and more popular, I expect we will see a lot more of this in the future :)
resolved and released in 1.1.1
Thanks folks. Just tested by updating the version in the toolkit and this issue seems resolved.
However, now uuid is causing the same issue. Even though it has been around for a while - I believe #840 added it to the critical path for the browser recently. I did verify it's uuid by removing uuid from the core project.
Looks like there is an experimental PR open for uuid (from the author) to add es modules (https://github.com/kelektiv/node-uuid/pull/317). Should I create an issue for this?
yes please do create an issue. I was wondering why that one wasnt causing an issue, but looks like it is
cc @jasonnutter
Created #874 and added a minimal repro project
@jasonnutter thank you open my issue #985
I am getting base64 decode error on 2 account.
1 account AAD, 1 account LiveID.
this issue is serious.
Service unavailable due to can not user signin.