OfficeDev / microsoft-teams-library-js

JavaScript library for use by Microsoft Teams apps
https://docs.microsoft.com/microsoftteams/platform/
Other
430 stars 199 forks source link

Support SSR applications #602

Closed thure closed 1 year ago

thure commented 3 years ago

Not all Teams apps are SPA's - crucially, a project which supports server-side rendering will encounter build errors if this package is loaded since window is not defined.

This package should at least return some sort of default value instead of erroring when window is not defined.

ghost commented 3 years ago

Hi thure! Thank you for bringing this issue to our attention. We will investigate and if we require further information we will reach out in one business day. Please use this link to escalate if you don't get replies.

Best regards, Teams Platform

gablabelle commented 3 years ago

We are facing the same issue. We are using next.js and in order to get pass these errors we have to make sure the library is only used client side.

@thure to get pass this, you can lazy import/require the library in a useEffect hook because it only runs client side or if you use next.js, use their dynamic component and disable ssr as per the next.js documentation.

Meghana-MSFT commented 3 years ago

@thure Could you please share a sample or minimal code snippet for us to repro the issue?

gablabelle commented 3 years ago

@thure Could you please share a sample or minimal code snippet for us to repro the issue?

Here you go https://stackblitz.com/edit/nextjs-avwbvb?file=pages%2Findex.js

Let stackblitz run the app install and display the working app on the right... Then uncomment the commented code below the TODO commnent and see that @microsoft/teams-js makes the app crash because it's trying to access window which doesn't exist when the code is running server-side (on node.js, on next.js).

The @microsoft/teams-js doesn't support SSR because of that.

Meghana-MSFT commented 3 years ago

microsoft/teams-js works in the context of Microsoft teams. Please try it out in teams context and it should work for you.

gablabelle commented 3 years ago

microsoft/teams-js works in the context of Microsoft teams. Please try it out in teams context and it should work for you.

No @Meghana-MSFT you are wrong ... If the app embedded in the Teams iframe is using server-sider rendering it will break the same way even in the context of Teams.

This is because the library is trying to access the window object that doesn't exist server side. A check should be done prior to trying to access the window object.

ryan11223344 commented 3 years ago

I have started developing a teams app for my group and I am running into the same problem, @gablabelle what did you do to bypass it for now until a patch can be made??

gablabelle commented 3 years ago

I have started developing a teams app for my group and I am running into the same problem, @gablabelle what did you do to bypass it for now until a patch can be made??

@ryan11223344 You have to lazy load Teams client SDK within a useEffect hook because useEffect only runs client-side so you won't get the server-side error.

Something like the following in a React Functional component should work.

const [teamsClient, setTeamsClient] = useState();

useEffect(() => {
    const loadTeamsClient = async () => {
      const client = await import('@microsoft/teams-js');
      setTeamsClient(client);
    };
    loadTeamsClient();
}, []);
Meghana-MSFT commented 3 years ago

@thure

Did you try any of the solutions and did it work out for you?

shaohaolin commented 2 years ago

+1 on this issue. This also breaks on any SDK that has dependencies on microsoft/teams-js and then the SDK is used in any SSR applications.

thure commented 2 years ago

@thure

Did you try any of the solutions and did it work out for you?

@Meghana-MSFT I'm not (currently) developing a Teams app, so I did not try any of these solutions myself, but I don't think it's relevant whether there are workarounds, IMO the original issue is significant and should be fixed.

Given that this package doesn't need the window API to deliver strictly all meaningful functionality, it’s definitely an implementation flaw that this package can't run on the server.

If folks really need SSR with this package, it's all open source, someone should submit a PR.

ChetanSharma-msft commented 2 years ago

Hello Everyone, Sorry for the late response. Please let us know if you need any further help here or shall we close the issue?

gablabelle commented 2 years ago

Hello Everyone, Sorry for the late response. Please let us know if you need any further help here or shall we close the issue?

Why close an ongoing issue? In 2022, with almost all the FE frameworks now implementing SSR, this issue should be addressed IMO.

Screen Shot 2022-07-25 at 10 06 13

https://twitter.com/kentcdodds/status/1531710215762104323

This design flaw is super simple to address, the SDK should check if window exists before trying to access it to prevent it to break SSR.

ChetanSharma-msft commented 2 years ago

@gablabelle - Thanks for your response. We will share your feedback/suggestion with engineering team & let you know the updates.

As an alternative, You can also raise your feedback here: https://feedbackportal.microsoft.com/feedback/forum/ad198462-1c1c-ec11-b6e7-0022481f8472

ghost commented 2 years ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 3 days. It will be closed if no further activity occurs within 3 days of this comment.

gablabelle commented 2 years ago

Quick message to prevent the issue from closing.

ghost commented 2 years ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 3 days. It will be closed if no further activity occurs within 3 days of this comment.

nwojod-MSFT commented 2 years ago

There is a backlog item for this issue, but no ETA for when it will be fixed. However, we are open if anyone from the community has a proposed fix and would like to submit a PR for it. Otherwise, we will update this thread when there is an ETA.

GregfromKnowork commented 2 years ago

Bump. Major blocker for our Teams app.

nwojod-MSFT commented 2 years ago

@GregfromKnowork sorry to hear it is a major blocker for you and your team. We will update this post when we have ETA for a fix.

DopamineDriven commented 2 years ago

Huge blocker for my org. I have an embedded microsoft 3DVista Virtual tour "Tech refresh Store" with Azure AD auth wrapping it using Nextjs + NextAuth + an edge runtime middleware auth wrapper that works like a charm in the browser and when the teams app is opened in the browser. The issue is that the vast majority of modern apps are isomorphic and use both server and client rendering.

Basically, from the stable release of react 18 on this will be an ongoing issue for many enterprise teams simply because react 18 is all about embracing isomorphic runtimes and the benefits of SSR components etc

Stripping the ability to do concurrent streaming, to have node and edge runtime environments for the elimination of JS overhead by rendering some components on the server instead of in the client, and so on results in a less than ideal Developer experience. Not to mention it is inherently safer to execute auth-flows server side not client side. so perhaps this should be reprioritized by the internal team given that SSR rendered React apps are only going to increase in frequency, not vice versa like we saw circa 2019.

perhaps adding a conditionally invoked node runtime config to support SSR within teams would be a solution? Docker?

nwojod-MSFT commented 2 years ago

Hi @DopamineDriven sorry to hear this is a huge blocker for your org. We understand the importance of this feature, however, we are examining it with other features in our backlog and prioritizing accordingly.

nwojod-MSFT commented 2 years ago

We understand the importance of this feature and have added it to our backlog. The team will review it, the level of work required, and prioritize accordingly. Because of the volume of requests we receive, we are unable to promise when this feature will be implemented. The earliest we will prioritize this is first half of next calendar year. We will provide an estimate on this thread as soon as we have a committed timeline.

We welcome any contribution from the community on a fix. Thank you for your patience.

bsana1 commented 2 years ago

The workaround proposed by gablabelle, to dynamically load the library on the client side works, however, I wanted to express my disappointment with MSFTs responses here. This was happening in version 1.x of team-js, and while a hack exists, the real fix is literally, simply, enforcing a check for the existence of the window object in code, prior to using the global variable:

if(typeoff window !== 'undefined) { // do something }

It's beyond my comprehension to understand why so much pushback on fixing it, and the amount of effort put to extend this thread indefinitely without a fix, really sad customer support service provided and poor engineering effort.

webdes03 commented 2 years ago
const [teamsClient, setTeamsClient] = useState();
useEffect(() => {
    const loadTeamsClient = async () => {
        const client = await import("@microsoft/teams-js");
        setTeamsClient(client);
    };
    loadTeamsClient();
}, []);

I've got this working, but teamsClient is just an any type so it's a nightmare to work with. Is there a way to lazy import the types as well, or somehow reference the types externally and I just don't know how to do it? Maybe @gablabelle or @bsana1?

TrevorJoelHarris commented 2 years ago

Hi everyone! Sorry for the delay on helping out here. We'll actually be going in and making the fix @bsana1 mentioned above in the next couple of months. Part of the reason this change has taken a while to get to is that while making the fix @bsana1 mentions above is very straightforward, we also need to add some robust tests to make sure that no one inadvertently breaks SSR apps that use this library in the future.

bsana1 commented 1 year ago

Thanks for that, and for following up!

webdes03 commented 1 year ago

@TrevorJoelHarris is there any update on this fix or any idea where it sits in the prioritization? We've got numerous custom apps and products in the pipeline and want to move to an SSR model but are kind of stuck at the moment.

TrevorJoelHarris commented 1 year ago

@TrevorJoelHarris is there any update on this fix or any idea where it sits in the prioritization? We've got numerous custom apps and products in the pipeline and want to move to an SSR model but are kind of stuck at the moment.

@webdes03 We are hoping to ship a fix for this in our next version (2.8) that will be coming out sometime next week. We are still validating that the fix works correctly, but assuming all goes well there we should be good to go.

ryceg commented 1 year ago

Is there any fix on this yet?

TrevorJoelHarris commented 1 year ago

Hi @ryceg, unfortunately getting validation in for this has taken longer than we were expecting for a bunch of boring reasons. We're just about finished, and are planning to ship a fix by the time the next official release comes out on 4/5. Sorry for the inconvenience!

noahdarveau-MSFT commented 1 year ago

Hello everyone, the fix for SSR has been checked in and will be available beginning with our next release of version 2.10.0 on 4/5.

webdes03 commented 1 year ago

Thank you @noahdarveau-MSFT. I'm slow to follow up here, but can confirm teams-js working in nextjs with SSR. Thanks!

ghost commented 1 year ago

Tell us about your experience!

Hi thure! This issue is closed in our system. We would like your feedback on your experience with our support team and Platform.

Best regards, Teams Platform

guillewu commented 1 year ago

@webdes03 could you share what do you mean that is working in nextjs with SSR? I found this example: https://github.com/OfficeDev/microsoft-teams-library-js/blob/main/apps/ssr-test-app/pages/index.tsx#LL16C4-L16C4 and it seems like it's still initializing the library client side. If you have any tips, I'd appreciate it. Thanks!

thure commented 1 year ago

@guillewu, React runs in both the client and the server. The test page you cite is runs in both environments, and you would see by running ssr-test-app that the server proves it by rendering slightly different content using the same code. As far as I can tell, ssr-test-app is just a test case that SSR works, not an example of how you would set this up with Next.js.

The readme is fairly informative: https://github.com/OfficeDev/microsoft-teams-library-js/tree/main/apps/ssr-test-app

webdes03 commented 1 year ago

@webdes03 could you share what do you mean that is working in nextjs with SSR? I found this example: https://github.com/OfficeDev/microsoft-teams-library-js/blob/main/apps/ssr-test-app/pages/index.tsx#LL16C4-L16C4 and it seems like it's still initializing the library client side. If you have any tips, I'd appreciate it. Thanks!

I just used npx create-next-app@latest to create a Next solution from scratch, then created a page in pages/tabs/app/config/index.tsx to model a configuration page for a tab app. I'm using a fork of the useTeams hook at https://github.com/wictorwilen/msteams-react-base-component inside the page to fetch the teams context and return it to my page. If it seems like it's just stuck, make sure you're calling notifySuccess to tell the Teams SDK your page loaded.

guillewu commented 1 year ago

@thure @webdes03 thanks for your quick reply. I just moved some code to use SSR, ie:

import * as teamsjs from "@microsoft/teams-js";
const { authentication } = teamsjs;
const token = await authentication.getAuthToken();

And this no longer works as it's happening server side. Would you guys happen to know how to obtain a token now SSR? TIA

webdes03 commented 1 year ago

I've always uses OBO in a server route to get tokens- in that pattern you'd get an identity token client side, and then hand off to an API route to convert that into a scoped access token.

ricardasjak commented 1 year ago

@webdes03 , @guillewu would you mind to share some example how to pass client side access token acquired by teamsjs to next-auth?

webdes03 commented 1 year ago

I've actually never used next-auth. I always build my Teams apps to use SSO so by the time they get to my app they've already been let through the App Service's auth layer, and I only need the access tokens to call services like Graph. I just do the OBO flow server side instead of messing about with middleware. Does next-auth support OBO (I'll admit, I've never looked)?