-
Notifications
You must be signed in to change notification settings - Fork 101
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.
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
Make useable in all js environments #334
Comments
@morenoh149 oh that's interesting, I don't think we've seen that one before. Thanks for the find! |
yep it is because of server-side rendering, you have lines like Workaround may be using branch only on the client, use async chunk for example |
Just ran into this as well... 😢 |
Just ran into this as well... |
IS this still an issue? trying to implement in React project and getting the same error only on "import" of the package... Anyone have a workaround? |
This is completely blocking for server side rendered apps. What is the recommended workaround? Could the global be mocked just so the import works on the server? |
As a workaround I'm lazy loading the branch library using |
I've handle it with isomorphic-ensure with |
Any plan on fixing this? this is a blocker |
Hey Everyone, Our Product and Technical teams have decided to not move forward with this feature request. Reason being that the WebSDK depends on information from the browser for most of our SDK features. We lose that information if it runs server-side instead. We apologize for any inconveniences caused as a result of this decision and we look forward to servicing your needs through the browser environment. |
@rubinsingh This has nothing to do with running branch code on the server, the problem is that you can't even import the branch sdk module on the server side which makes your developers jump through hoops to use your module. I'll even save you guys the work, here is the code:
|
Indeed, @rubinsingh if you read the first post, if a developer wants to use branch in a worker thread (that's still in the browser) they have to go through hoops. It's just bad javascript hygiene to assume the code is being run with the |
@rubinsingh I concur. Like the others, our use case is not to use the SDK from within Node on the server side but to allow the inclusion of the module there for server side rendering a React app. The work around for this hurts the user experience and is very much not ideal for both the user and the developer. |
I don't think it's trivial to make the sdk not depend on the window object. @grydstedt I just peeked through https://github.com/BranchMetrics/web-branch-deep-linking/search?p=2&q=window&type=&utf8=%E2%9C%93 The change is going to require making a module for this project (standard practice) and update all refs to
it's very presumptuous to use @rubinsingh It's fine if you all can't address this in a certain amount of time. Simply recognize it as tech debt and add it to your Backlog. Add a |
Hey Team, Thank you for your feedback. To get our WebSDK operable in environments where the window object is not available is a significant undertaking for the team (as you've seen through searching for references to the 'window' object). We've found another area that that would need to be re-tested/potentially re-written as a result of this change. That area is related to storage i.e. how we use application, local and cookie storage in the WebSDK to store and retrieve items related to our core deep linking functionality. We do understand your frustration though so we've added this as a feature request in our internal WebSDK backlog. Incase anyone in this thread finds it useful, we have an HTTP API as an alternative: https://github.com/BranchMetrics/branch-deep-linking-public-api @pho3nixf1re regarding using Branch with a server side rendered react app, I wanted to share a technique I used to make Branch work in this environment. Using the example at the following location: https://github.com/mhart/react-server-example I made the following changes:
at: https://github.com/mhart/react-server-example/blob/master/App.js#L18 This worked well for me. Please feel free to shoot an email to [email protected] if you're having trouble with React and Branch. @morenoh149 thanks for calling out the .sdk_version issue - we've added it to our backlog. |
@rubinsingh it doesn’t matter if it operates; it still should be requireable in a non-browser environment. This isn’t a feature request, it’s a bug. Everything should be importable/requireable in every environment, whether it’s going to work or not. In airbnb’s case, all react components are server-rendered, and browser-only things are invoked only in componentDidMount or event handlers. |
Thanks everyone for the thoughtful discussion on this issue! I will talk with our Product team to figure how best to support this use case. I completely understand that in its current state, our web SDK might not be ideal for all web applications, especially ones that might be rendering part of the page server-side or a hybrid app of sorts, due to its dependency on the window object. As Rubin mentioned above, the changes required to remove the dependency on the window object will not be quick. We will need to decide on the best approach to this work, to make sure we keep all functionality and features available for customers that are already using this SDK regularly, and many of those currently rely on the existence of the windows object and will not work without it. This issue has been backlogged and I will update this thread as soon as I have more details to share. In the meantime if you have any further questions or concerns, please let me know. Or you can submit a ticket with our support team: https://support.branch.io/support/tickets/new/. |
It’ll take a few ternary statements and if blocks to make it work identically in a browser, but fail gracefully elsewhere. I wouldn’t expect it would take more than a day’s work, and that’s being extremely conservative. |
When I ported my team's code to an SSR environment, I ran up against this. Since we don't care about Branch on the server, I was able to get around this by doing the following. We have a const branch = (EnvironmentUtils.isBrowser())
? require('branch-sdk')
: null; The usage of the methods from the const BranchUtils = {
init(key, cb) {
if (!branch) { return; }
branch.init(key, cb);
},
link(data, callback) {
if (!branch) { return; }
branch.link(data, callback);
},
setViewData(data) {
if (!branch) { return; }
branch.setBranchViewData(data);
}
}; Hopefully that helps some people out until the Branch team can fix this internally. |
The branch sdk should check if window exists before utilizing it. See https://github.com/BranchMetrics/smart-app-banner-branch-deep-linking-web-library/blob/master/dist/build.js#L1845
This was only discovered while running branch in a worker thread, as
window
is not available in that environment.The text was updated successfully, but these errors were encountered: