Skip to content
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

Not usable in NodeJS #876

Closed
sallar opened this issue May 21, 2018 · 24 comments · Fixed by #3902
Closed

Not usable in NodeJS #876

sallar opened this issue May 21, 2018 · 24 comments · Fixed by #3902
Labels
Core Related to core Amplify issues feature-request Request a new feature

Comments

@sallar
Copy link

sallar commented May 21, 2018

Do you want to request a feature or report a bug?
Bug Report

What is the current behavior?
Throws an Error when using in NodeJS

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than AWS Amplify.

  1. Require this module in NodeJS
  2. Try to use it

What is the expected behavior?
The codebase should not have direct references to window object at all. This is considered bad practice. Authentication for example, is something one might want to use in NodeJS instead of browser or RN. There should be getters instead of accessing window, so they can be polyfilled when necessary.

Also if you're not going to support NodeJS, please kindly specify this in the docs or README, right now the docs are very vague about this matter.

Thank you ♥️

@mlabieniec
Copy link
Contributor

@sallar, we have included some polyfills that polyfill the window object so that it is usable in node. The only other thing you would need to do is include a fetch module i.e. node-fetch for the fetch requests done by the amazon cognito sdk, see here: #403

@sallar
Copy link
Author

sallar commented May 21, 2018

@mlabieniec thanks. But the polyfill doesn’t seem to work. As soon as you import the module it throws an error. Is there any example of usage in node?

@sallar
Copy link
Author

sallar commented May 21, 2018

The errors appear to come from importing everything from RNComponents folder

@manueliglesias manueliglesias added the investigating This issue is being investigated label May 22, 2018
@wzup
Copy link

wzup commented Jun 20, 2018

Same error
#1066

@flaviocordova
Copy link

I have a really serious issue related to Polyfills creating a global.window object.

My project uses NuxtJS (a framework based on VueJS) using SSR (server-side render) and, as the name suggests, some pages are rendered on server before being sent to client.

When running in DEV mode I can render the page when first loaded (when global.window is undefined) because the engine can identify when it's running on the server. But if I change a component and it triggers the rebuild process, it fails (because global.window is now an object)...

besides that, I had to remove some components because they can't identify when they are on server...

By now I'm mostly interested on Auth/Cognito (although storage is something I'll need in a near future)... is there any workaround I could use to avoid the global.window creation?

@elorzafe
Copy link
Contributor

elorzafe commented Jul 18, 2018

Hi @flaviocordova I have checked on node.js an App that uses Auth and is working if you install node-fetch
and then add global.fetch = require('node-fetch'); before importing Amplify.

@flaviocordova
Copy link

Hi @elorzafe, thanks for your prompt reply...

My problem (or anyone with a vuejs ssr application) is not that nodejs should mock the browser environment better but instead should not do that at all..

The point here is that amplify should not create a window object because it's making the server-side code to misbehave.

take a look at https://github.com/vuejs/vue/blob/3eb37acf98e2d9737de897ebe7bdb7e9576bcc21/src/core/util/env.js, line 7 and you'll find it:

export const inBrowser = typeof window !== 'undefined'

When the app is first built (on dev mode) window is undefined, so the engine assume it's running server-side. Once amplify is loaded, window is now an object and all the components and requests will cause the server to behave like a browser.

I think amplify should not change the current environment to mock a browser but instead should just read it to identify if it's running in browser or in node and then decide behave accordingly. For example: I should not need to import node-fetch to mock the browser - amplify should do that automatically when it "senses" it's not running in the browser.

Doesn't it make sense?

@mlabieniec
Copy link
Contributor

mlabieniec commented Jul 18, 2018

@flaviocordova thanks for the detailed response. The point of this pollyfill was originally for simple support for amplify to run in vanilla node. We'll take a look at this and see if there is a way we can address this. Please feel free to provide suggestion/PR in the meantime.

@mlabieniec mlabieniec added feature-request Request a new feature and removed investigating This issue is being investigated labels Jul 18, 2018
@flaviocordova
Copy link

Hi @mlabieniec, thanks !!!

I understand the motivation to mock the browser, I just think thats not the best approach. But, sincerely, who never, right ? 😄

I don't know the code details but I'd consider removing polyfill and addressing the fetch issue first (since it's seems to be the reason why polyfill was created).
A facade to fetch method that checks if it should fetch directly (browser) or use node-fetch (or something else) should do the magic without big impact on the current source..

Do you have an idea of when polyfill was released ? I was wondering if using an old version could prevent the issue now until a permanent solution is released.

@mlabieniec mlabieniec added the Core Related to core Amplify issues label Jul 19, 2018
@flaviocordova
Copy link

Hi @mlabieniec...

I did some small changes locally to avoid the error and, so far, it seems to be working, at least for the authentication...

Basically, I tried to reverse the mimic making Polyfill to create a global variable instead of creating a window on node. Definitely not the best approach (as I said before, it'd be better to have some facades), probably not enough to work everywhere, but may help to throw some light to a final solution.

The changes bellow were made in the final code in "node_modules".. I have cloned the project and changed in TS but I was not sure how to replace the current version with the one I have built (sorry). Anyway, I believe they are good enough to illustrate the changes.

Polyfill.js

// Add a global variable in browser
if ((typeof window) === 'object' && window.global == null) {
    console.log("Mocking global...")
    window.global = {
        window: window // may be useful to make global.window to work everywhere, instead of just window
    }
}
// if (!global.window) {
//     global.window = {
//         setTimeout: setTimeout,
//         clearTimeout: clearTimeout,
//         WebSocket: global.WebSocket,
//         ArrayBuffer: global.ArrayBuffer,
//         addEventListener: function () { },
//         navigator: { onLine: true },
//         location: {
//             href: ''
//         }
//     };
// }

RNComponents/index.js

var AsyncStorage = global.localStorage || window.localStorage;

StorageHelper

var StorageHelper = global.localStorage || window.localStorage;

@sallar
Copy link
Author

sallar commented Jul 20, 2018

It's not possible to even polyfill. If you use the import amplify from ... syntax for importing the library, the library code runs before everything else (unlike require). So window.fetch = ... doesnt work. The best way is just to remove all references to it from the library.

I'll maybe submit a PR for this when I get a bit of free time.

@gustavo-rios
Copy link

I am working on a node CLI app (with Typescript) accessing Cognito via Amplify to get a temporary AWS Access Key once the user is authenticated and I am hitting the "fetch is not defined" issue, so far I was able to get away by modifying Client.js as proposed by doscode-kr in issue #403, using node-fetch, obviously, this is a temporary solution and not ideal.

@bugeats
Copy link

bugeats commented Sep 19, 2018

It's not just the window object. Here the lib assumes access to the navigator object without doing any checking:

DeviceName: navigator.userAgent,

You could go a long way by simply building your stuff in Node to discover these kinds of errors. There's no reason to have a hard dependency on navigator.

BTW those of you trying to get this work in Node, try something like this:

// stupid assumes global fetch
global.fetch = require('node-fetch');
global.navigator = {};

Make sure you leave the snarky comment in there so everyone knows of your displeasure.

@ispyinternet
Copy link

Please un-polyfill window in Node. Breaks my environment!!

@romainquellec
Copy link

Breaking a lot with next right now. I'm using noSSR for now, but it's not a viable solution at the end. 👍 for this issue.

@bingtimren
Copy link

bingtimren commented Oct 2, 2018

Hi if you just want to use AppSync service with node client I built an example project here.
https://github.com/bingtimren/aws-appsync-nodejs-template.js

Though it didn't use Amplify. Hope it helps.

@OtterFlip
Copy link

Using some of the aforementioned hacks I was able to get an older version of aws-amplify (0.4.8) to work with Node.js v10.3.0. However there are some bugs in that version of aws-amplify and I'd really like to upgrade to aws-amplify 1.1.4. However the same hacks don't work and I can't seem to get aws-amplify 1.1.4 working with Node.js.

Has anyone successfully nudged aws-amplify 1.1.4 to work with Node? If so, can you please explain how you did so?

Amplify Team: PLEASE make Amplify work with Node.js. This will be extremely helpful as it will enable us to use shared code for node test scripts as well as web and react clients.

@romainquellec
Copy link

I got this issue fixed with "1.1.6-unstable.2" but it does not with "1.1.9".

@closedLoop
Copy link

For those also using typescript on a node-js backend and are getting a ReferenceError: fetch is not defined error

npm install these packages:

    "amazon-cognito-identity-js": "3.0.11"
    "node-fetch": "^2.3.0"

Then include:

import Global = NodeJS.Global;
export interface GlobalWithCognitoFix extends Global {
    fetch: any
}
declare const global: GlobalWithCognitoFix;
global.fetch = require('node-fetch');

@stale
Copy link

stale bot commented Jun 15, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@homerjam
Copy link

Can you please make it clear on the website where these major gaps in functionality are that one would expect from a project like this? Or mark it as a beta product?

@aldegoeij
Copy link

referencing tickets #3486, #3506, #3615, #3015, #876, #3477 and webpack/webpack#6522

@talaikis
Copy link

Ia hev included core-js, etc. and it works fine, one of examples: https://www.npmjs.com/package/@talaikis/s3-db

@github-actions
Copy link

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core Related to core Amplify issues feature-request Request a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.