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

util.isNode / Error: not supported #1221

Closed
jumpinvestor opened this issue May 13, 2019 · 9 comments · Fixed by #1363
Closed

util.isNode / Error: not supported #1221

jumpinvestor opened this issue May 13, 2019 · 9 comments · Fixed by #1363

Comments

@jumpinvestor
Copy link

jumpinvestor commented May 13, 2019

protobuf.js version: 6.8.8
Ubuntu: 19.04
Node: 12.2.0
Angular: 7.2.15 with server-side rendering enabled

I'm seeing "Error: not supported" popping up in an Angular 7 app on Node 12.2.0. Just upgraded Node from 10.15.3, where it did not have the error.

Screen Shot 2019-05-13 at 3 35 06 PM

After a little digging, it looks like this references /src/root.js:233-234:

Screen Shot 2019-05-13 at 3 50 00 PM

and /src/util/minimal.js:55:

Screen Shot 2019-05-13 at 3 38 05 PM

After commenting out the check in root.js, everything seems to work fine. Is there another solution?

@dcodeIO
Copy link
Member

dcodeIO commented May 14, 2019

The check is here: https://github.com/protobufjs/protobuf.js/blob/master/src/util/minimal.js#L55

There might be something in your stack invalidating it, or did node change something? Workaround:

protobuf.util.isNode = true; // or your own check
...

@jumpinvestor
Copy link
Author

I think it might be related to Angular's server-side rendering. The util object captures global.document but not global.process, which could make sense for SSR:

Screen Shot 2019-05-14 at 8 23 34 AM

@MichaelSolati
Copy link

MichaelSolati commented May 16, 2019

I'm having a similar, so what I found worked (which I don't think would be a hard/serious change) is in src/util/minimal.js I reordered the util.global. It was:

util.global = typeof window !== "undefined" && window
           || typeof global !== "undefined" && global
           || typeof self   !== "undefined" && self
           || this; // eslint-disable-line no-invalid-this

And I swapped it to this:

util.global = typeof global !== "undefined" && global
           || typeof window !== "undefined" && window
           || typeof self   !== "undefined" && self
           || this; // eslint-disable-line no-invalid-this

I'm using mocha with jsdom so the window object is passing first and therefore being set as the global, not the NodeJS.Global! But if I swap the order it works fine. Could this be implemented into the regular library?

@dcodeIO
Copy link
Member

dcodeIO commented May 16, 2019

Hmm, good question. The old order assumes that a global window is super unlikely inside of a node module because there isn't an implicit global scope one could accidentally declare such a variable in, while the new depends on the assumption that no dev would ever define a global var named global in a browser context, which is unlikely.

@mfursov
Copy link

mfursov commented May 22, 2019

I believe this may be a conflict with NestJS people use a lot for SSR:
https://github.com/nestjs/ng-universal/blob/master/lib/utils/domino.utils.ts

If protobuf.js is initialized first, there is no error with isNode mode detection.

@jumpinvestor
Copy link
Author

That makes sense.

@MichaelSolati
Copy link

It's also an issue if you are using jsdom with mocha.

@michelepatrassi
Copy link

michelepatrassi commented Aug 19, 2019

EDIT: wrote an article about how I fixed this with angular SSR: https://medium.com/@michele.patrassi/angular-ssr-firebase-save-days-of-debugging-by-reading-these-fixes-fcb060e248bb

OLD
same, I had this with the latest version of firebase (currently 6.4.0). I needed to downgrade to "^5.9.1" to still have SSR working.

There are some valid comments up in the thread: what's the best way to approach this?

@diegoblattner
Copy link
Contributor

diegoblattner commented Feb 20, 2020

Several questions arise from this, but...

util.isNode = Boolean(process && process.versions && process.versions.node);

Fixes the problem, considering process is a global variable since node 0.1.7 and versions since 0.2.0
I will create a PR.

alexander-fenster added a commit that referenced this issue May 29, 2020
* Fixes #1221 when used within SSR.
`process` is a global variable since node [0.1.7](https://nodejs.org/api/globals.html#globals_process) and versions since [0.2.0](https://nodejs.org/api/process.html#process_process_versions).

Co-authored-by: Alexander Fenster <fenster@google.com>
taylorcode pushed a commit to taylorcode/protobuf.js that referenced this issue Oct 16, 2020
* Fixes protobufjs#1221 when used within SSR.
`process` is a global variable since node [0.1.7](https://nodejs.org/api/globals.html#globals_process) and versions since [0.2.0](https://nodejs.org/api/process.html#process_process_versions).

Co-authored-by: Alexander Fenster <fenster@google.com>
This was referenced May 20, 2022
renawolford6 added a commit to renawolford6/protobuf-script-build-javascript that referenced this issue Nov 10, 2022
* Fixes protobufjs/protobuf.js#1221 when used within SSR.
`process` is a global variable since node [0.1.7](https://nodejs.org/api/globals.html#globals_process) and versions since [0.2.0](https://nodejs.org/api/process.html#process_process_versions).

Co-authored-by: Alexander Fenster <fenster@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants