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
673 stars 95 forks source link

Window.location functions replaceState and pushState defined to null in OfficeJS #1344

Closed Medhyena closed 4 years ago

Medhyena commented 4 years ago

The library replaces some native JavaScript functions.

Expected Behavior

Not to replace native JavaScript functions except if it is an override to implement new features to it.

Current Behavior

Error when using window.location.replaceState or window.location.pushState. They are defined as null and thus, breaking other libraries using these native JavaScript functions.

Please do not replace such core functions to not break applications.

Steps to Reproduce, or Live Example

1: Create an addin with OfficeJS 2: Try window.location.replaceState (for example) 3: Application breaks.

Context

Our authentication system is using these to redirect the user to our application. It's basically impossible to reach the application at this point.

Your Environment

dbene commented 4 years ago

Duplicate of #429

Take a look at this workaround from stackoverflow.

I know it is awful that this is still not fixed, or at least added an exveption for Edge like they do for addins on android. See Line 1901

sumurthy commented 4 years ago

This is in the backlog to be fixed.

jcbdev commented 2 years ago

This works for me to fix in angular which manifests as a routing loop bug

<body>
    <app-root></app-root>
  </body>
  <script>
    // this late load is required due to bug in office-js causing a routing loop
    // https://github.com/OfficeDev/office-js/issues/1198
    // https://github.com/OfficeDev/office-js/issues/1344
    // https://github.com/OfficeDev/office-js/issues/429
    let replaceState = window.history.replaceState;
    let pushState = window.history.pushState;
    var head = document.getElementsByTagName("head")[0];
    var script = document.createElement("script");
    script.type = "text/javascript";
    script.onload = function () {
      Office.onReady().then(() => {
        console.log("Office is ready!");
        window.history.replaceState = replaceState;
        window.history.pushState = pushState;
      });
    };
    script.src = "/assets/office-js/office.js";
    head.appendChild(script);
  </script>
JimbobTheSailor commented 1 year ago

This issue has raised its ugly head again when I updated to Angular 16 and msal-angular 3.0.0-beta (which is required for angular 16).

My purpose in writing is to comment that the workaround as outlined by jcbdev is not an optimum solution as it causes asynchronous issues in the loading of Office.js and the subsequent use of its functions. If Office.js is not cached, then any code in <app-root></app-root> will execute before Office.js is loaded, resulting in the error "'Office' not defined when you try to use any Office.js functions. To make it work you would need to wire in additional code to wait for Office.js to be loaded into the DOM before bootstrapping your app.

Or, alternatively, you can keep everything in sync by modifying jcbdev solution slightly as below:

<head>
    <script>
        window.myReplaceState = window.history.replaceState;
        window.myPushState = window.history.pushState;
    </script>
    <script src="https://appsforoffice.microsoft.com/lib/1/hosted/Office.js" type="text/javascript"></script>
    <script>
        window.history.replaceState = window.myReplaceState;
        window.history.pushState = window.myPushState;
        delete window.myPushState;
        delete window.myReplaceState;
    </script>
<head>
gergzk commented 7 months ago

Hi everyone, The dev team has recently made a fix for this issue (removed the override with null). Expect to see it roll out to CDN in the next 1-2 weeks.

JimbobTheSailor commented 7 months ago

Great news

torgestehr commented 6 months ago

That's really great news. Are there any updates regarding the rollout to CDN?

RK1PF commented 3 months ago

That's really great news. Are there any updates regarding the rollout to CDN?

We are in July 2024 and still no rollout. I was waiting for this change for 2 years, I was initially using Blazor to develop office add-ins but this "issue by design" made me change to React (without any framework or router). Today we are using NextJs in almost every of our projects and can't use it for office add-ins projects.