OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
648 stars 92 forks source link

There is an error “Mismatched anonymous define() module” when using Requirejs and Office.js #48

Closed magnusdanielson closed 6 years ago

magnusdanielson commented 6 years ago

In my Word AdIn I load two javasript files:

  1. office.js
  2. mybundle.js, includes requirejs In mybundle.js I use requirejs to load modules from my bundle. In most cases the error is thrown when the add in loads. image

Expected Behavior

I expect office-js not to interfere with using requirejs, one of the most common module loaders available.

Current Behavior

When word-win32-16.0.1.js. loads, it inserts a script tag for aria-web-telemetry.js which is the culprit of the error. The anonymous define is called at line 1, column 134 in aria-web-telemetry.js. The last line in the file is //# sourceMappingURL=aria-webjs-sdk-1.1.1.min.js.map if it guides you to the specific version of the file.

Steps to Reproduce, or Live Example

Create a Word AddIn project from Visual Studio 2017 Create a bundle file of significat size ~1Mb which includes requirejs. Add a script reference to office.js Add a script reference to the bundle file Error is thrown when add-in is started. Same issue is described here https://github.com/OfficeDev/office-js-docs/issues/1181#issuecomment-357392119

Context

I try to use requirejs to load my modules. The add-in does not work at all due to this error. I have a temporary work-around. I wait until all scripts are loaded on the page, including aria-web-telemetry.js and then insert a script tag to my bundle. In that case requirejs loads fine and my add-in works as expected.

Work-around <script> function load() { console.log("load event detected!"); var ausc = document.createElement('script'); ausc.src = '@Url.Content("~/Scripts/vendor-bundle-prewrap.js")'; ausc.type = 'text/javascript'; ausc.setAttribute("data-main","aurelia-bootstrapper"); document.getElementsByTagName('head')[0].appendChild(ausc); } window.onload = load; </script>

The easiest solution would for you to just name the module, instead of beeing anonymous. Read more about the error message here http://requirejs.org/docs/errors.html#mismatch

Your Environment

Platform PC Host Word Office version 2016 Operating System Windows 10 Browser Chrome Office-js file url https://unpkg.com/@microsoft/office-js@1.1.2-beta-next.0/dist/office.js

Zlatkovsky commented 6 years ago

Thanks for reporting! We actually had a discussion recently about aria-web-telemetry, so your report is well-timed. I will being it up to the team.

ndeeH commented 6 years ago

Hi @Zlatkovsky , my add in just was rejected by the store validation because of this error! On Windows clients we get this error from time to time depending on load timings of the dependant modules (a reload mostly does load the add in properly). On Mac clients the loading of modules seems to be different and the store tester does not get the app to start (require.js missmatch error). So PLEASE name the define in aria-web-telemetry to avoid us doing those ugly workArounds. TIA, Andi

Zlatkovsky commented 6 years ago

@weird-dreams , let me re-ping one of my colleagues, who might be able to help with this.

IanPearce commented 6 years ago

Hi Any news on this or a beta version, I am currently using 16.0.9010.1000 and I get exactly the same issue. Kind regards Ian

ndeeH commented 6 years ago

@Zlatkovsky Please set a named define in the aria-web-telemetry.js to avoid those work arounds!!! The work around described by @magnusdanielson also has performance issues on caching.

trmini commented 6 years ago

Hi @weird-dreams and @magnusdanielson,

We have a proposed fix that I would like to verify myself. However, I can't repro the issue in the first place. Is this possible if you can send me the test bundle that repro the issue? I tried to reference office.js via script tag and load it using require.js but neither case repro. Perhaps, I didn't have the same loading sequence to trigger the conflicts, so your repro file will help.

Thanks, Trang

ndeeH commented 6 years ago

@trmini I will send you a mail with manifest files to reproduce.

magnusdanielson commented 6 years ago

@trmini I have sent you my manifest.

trmini commented 6 years ago

Thanks @weird-dreams and @magnusdanielson. I was able to repro and verify the fix using your samples. I will work on getting it deploy in the next few weeks.

Thanks, Trang

ndeeH commented 6 years ago

Thank you @trmini . Good job!

IanPearce commented 6 years ago

Hi @trmini , any news on it going out?

trmini commented 6 years ago

Hi @IanPearce, we deployed the update to our test environment but was behind some unrelated changes before we can get to production CDN. In the mean time, you can test out the fix by referencing the Office.js here: https://appsforoffice.edog.officeapps.live.com/lib/1.1/hosted/office.js

Thanks, Trang

ndeeH commented 6 years ago

Hi @trmini, double checked your fix in our app - it works! Could you please notfify me, when I can remove the work around? TIA, Andi

IanPearce commented 6 years ago

Hi @trmini , I just checked too, it loads perfectly now, can you post on here when it goes live? Thanks for your help Ian

trmini commented 6 years ago

I verified that the change has been deployed to Production. Thanks again for reporting the issue and working with us to get it resolved.

Thanks, Trang

anoop7181 commented 6 years ago

Hi @trmini ,

Any idea when this update will be pushed as latest NuGet Package.

Thanks Anoop

ndeeH commented 6 years ago

Hi @anoop7181 , I think "aria-web-telemetry.js" is always loaded on demand from an CDN and therefore will not be part of a nuget package. Is this true @trmini ? see https://github.com/OfficeDev/office-js/search?utf8=%E2%9C%93&q=aria&type=

anoop7181 commented 6 years ago

Hi @weird-dreams ,

I am not quite sure if the is only issue with aria-web-telemetry.js . The latest Office js nuget still throws error. When i refer to Dogfood link of Officejs https://appsforoffice.edog.officeapps.live.com/lib/1.1/hosted/office.js) , i don't face the issue anonymous define module.

Just wondering when that update would be pushed to NuGet.

Regards Anoop

trmini commented 6 years ago

Hi @anoop7181,

We are working on updating the Nuget package with the fix. I will update the thread when it is ready.

@weird-dreams, a copy of the Aria SDK is part of the package as Office.js loads it from a relative path.

Thanks, Trang

anoop7181 commented 6 years ago

Hi @trmini ,

Thank you for the response. I will await update from you.

Regards Anoop

anoop7181 commented 6 years ago

Hi @trmini ,

Just checking again if there is an ETA for pushing the hosted prod update of officejs to NPM or nugget installer. I couldn't find latest files in Gtihub.

My customer has concern using it from CDN and our go-live is coming soon. Please advise.

Regards Anoop

trmini commented 6 years ago

Hi @anoop7181,

Unfortunately, there are some other dependencies that prevent us from publishing a new NugGet package right away. In the mean time, is this possible for you to override the ariatelemetry\aria-web-telemetry.js file with https://appsforoffice.microsoft.com/lib/1/hosted/ariatelemetry/aria-web-telemetry.js?

Thanks, Trang

magnusdanielson commented 6 years ago

@trmini @anoop7181 No that is not possible. aria-web-telemetry is loaded by Word not by us. We have no control over that, we don't use it at all. It is an internal Microsoft file that sends telemetry data about usage to Microsoft.

Zlatkovsky commented 6 years ago

I think what Trang meant was that if you are OK using an NPM package, then at that point you have flexibility to twiddle with this NPM package when you publish out your website (and also publish the contents of the NPM package, that is -- Office.js is not modularized/bundleable today, so essentially you'd just be hosting your own copy of what is on the CDN, modulo the one tweak).

That being said, using a non-CDN reference is not currently allowed in the Store, so it would be more of a development-time workaround rather than a workaround you can ship to the Store (under current Store policies). Hopefully we should be able to get the fix out onto the CDN soon.

eblancperso commented 6 years ago

I know this issue is closed but, do you have any ETA when this fix will be available through npm? I've tested with the CDN and it works but our configuration requires the npm package.

Thank you!

trmini commented 6 years ago

Hi, We have updated the NuGet package with the change: https://www.nuget.org/packages/Microsoft.Office.js/

Thanks, Trang

eblancperso commented 6 years ago

@trmini Thank you for your answer, but what about the npm package?

Zlatkovsky commented 6 years ago

Sorry for the delay. An update to NPM is forthcoming, I am working on it (and, more importantly, on making it trivial to publish NPM at the same time as our CDN in the future). I hope to have it ready in the next few days.

Zlatkovsky commented 6 years ago

@eblancperso & others on this thread: I'm happy to say that the NPM package is now updated.

You can get the latest Office.js version using “npm install @microsoft/office-js”! This will install version 1.1.6 which, today, is the latest version of office.js, corresponding to what you'd get on the public CDN today.

I’ve also updated our internal build process so that future official-version creation is simple. That way, we'll be able to keep the CDN and NPM package in sync.

Thanks for your patience on this.

eblancperso commented 6 years ago

@Zlatkovsky Is it normal that the definitelytyped "office.d.ts" in the @ microsoft/office-js package is not as up-to-date as the definitelytyped "index.d.ts" in the @ types/office-js package.

If you look at the definitelytyped from @ types/office-js, you can see that there is a method named "getSelectedEntities(): Entities;" from Mailbox 1.6, which can be found in the "outlook-win32-16.02.js" file under @ microsoft/office-js. This method is not in the definitelytyped under @ microsoft/office-js. The definitelytyped is not up-to-date within its own npm package?

eblancperso commented 6 years ago

@Zlatkovsky I'm sorry to poke you again but is it possible to have an answer about my previous question please? Thank you!

Zlatkovsky commented 6 years ago

@eblancperso , sorry, I somehow had missed your previous question. Thanks for re-pinging me on it.

It's a good question. In general:

  1. The d.ts file published on @types/office-js, https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/office-js/index.d.ts, @microsoft/office-js/dist/office.d.ts, and http://appsforoffice.microsoft.com/lib/1/hosted/office.d.ts should all be near-identical. I say "near" because there is a bit of a timing difference between how those are released, but they should be very very similar.
  2. For the host-specific APIs for Excel, Word, OneNote, and Visio, there is a pretty-much-guaranteed match-up between the implementation and the d.ts file, because the TypeScript is auto-generated, and the d.ts is thus auto-generated off of it. Unfortunately, this is not the case with Outlook, where the d.ts is written manually, and separately from the underlying implementation. As such, there can be cases where the two diverge. If you see this happen, please file a bug on https://github.com/OfficeDev/office-js/issues (or feel free to submit a pull request to https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/office-js/index.d.ts if you know what the right fix should be). In either of those cases, whether in an issue or a pull request, please give a link to the docs, so that we can figure out where the mismatch happened -- and I can pass it along to that team.

Thanks, and apologies for the mismatch,

~ Michael

eblancperso commented 6 years ago

Thank you @Zlatkovsky for your response! Quick remark here about the latest definitely typed package (under @ types/office-js). My web project is currently setuped to target ES5. When I try to use the definitely typed under @ types/office-js, I get an error about PromiseConstructor. I switched my project's typescript build target to ES6 and it seems to work. Isn't it a bit odd that you use a ES6 feature (PromiseConstructor) from lib.es6.d.ts for Office Add-In which doesn't support ES6 since it runs under IE11. The solution we found now is to target ES5 but specify lib ES6 so our project can build (the problem with that is that we can write/compile ES6 features like async, promise, etc but they will not work on runtime).

Zlatkovsky commented 6 years ago

@eblancperso , thanks for bringing up the PromiseConstructor issue. Let me look into it more. Opening https://github.com/OfficeDev/office-js/issues/150 to track this. If you have suggestions in the meantime on how else to author the d.ts file to achieve the same result but without the ES6 problem -- I'm all ears :-)!