Skip to content
This repository was archived by the owner on Aug 7, 2021. It is now read-only.

Do not set process to undefined #921

Closed
edusperoni opened this issue May 31, 2019 · 14 comments
Closed

Do not set process to undefined #921

edusperoni opened this issue May 31, 2019 · 14 comments
Assignees
Labels

Comments

@edusperoni
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently, webpack sets process to undefined

            new webpack.DefinePlugin({
                "global.TNS_WEBPACK": "true",
                "process": undefined,
            }),

which breaks plugins like nativescript-nodeify that use browser versions of library in NS.

Describe the solution you'd like
Remove "process": undefined, or explain the reasoning behind it.

Describe alternatives you've considered
Manually disabling it, but I don't know what are the reasons it was set like that in the first place.

In my tests, commenting the line had no side effects besides making everything that was broken work.

Additional context
I'm developing a remote abstraction for @ngrx/store-devtools. This means using browser libs that do checks on process.browser and libs that use require('ws'). I managed to replace require('ws') with nativescript-websockets, but there isn't a way to use process without changing the default webpack behavior, and the reason for it doesn't seem to be listed anywhere besides "process is node specific" (NativeScript/NativeScript#3080 (comment))

@DimitarTachev
Copy link
Contributor

Hi @edusperoni,

Thanks for the feedback!

If we do not define process as undefined, Webpack is setting it with a polyfill and the process object is available at runtime. In this way, some of the NativeScript plugins assume that they are used in a NodeJs environment (checking for the process object) and call some NodeJs specific APIs which are throwing errors and crashing the NativeScript app.

In other words, the NativeScript app is not a NodeJs environment and for this reason, we are removing this NodeJs specific object.

As a workaround, when using nativescript-nodeify, you could manually remove this process definition from the webpack.config of your app.

P.S. We are thinking about a way of extending the webpack.config in the plugins. When we add this feature in some of the future NativeScript versions, this could be directly handled in the nativescript-nodeify plugin.

@edusperoni
Copy link
Contributor Author

Hi @DimitarTachev! Thanks for the time for reviewing this.

There are some npm packages that work by checking process.browser. Maybe we could use a polyfill that uses process/browser, like nativescript-nodeify. I currently haven't run into any issues by removing "process": undefined

@NathanaelA
Copy link

@DimitarTachev - I just actually ran into this myself and this ends up being a breaking change from plain NativeScript (w/o webpack) in one of my plugins.

I just tested this, and this worked for me -- instead do this:
"process": "global.process"

This actually does work; and does allow other plugins to pollyfil the process properly... At least in my case I do actually use global.process to setup everything... So if it isn't already set, I create it -- if it does exist then I just add my additional functions if they don't already exist...

@edusperoni
Copy link
Contributor Author

@NathanaelA I suggested the polyfill for "safety" but I actually did not need it ¯\(ツ)/¯ Simply commenting the line "process": undefined was enough.

That line makes sure webpack will always evaluate process as undefined, which means no polyfill or augmenting will work. If the team thinks it's required to be undefined, maybe this should be set in the globals for tns-core-modules and let the developers override it?

@NathanaelA
Copy link

If you output the keys on process in a NS app after commenting out the line; you will see if has things on it that don't belong... So I can see why that added the line.

However; it does break polyfills like you ran into... After trying multiple things; what actually worked for me was the "process": "global.process" which basically is exactly what everyone should want as global.process should actually match process in a normal NS app. ;-)

@edusperoni
Copy link
Contributor Author

That's really weird behavior, I assumed process was already pointing to global.process. Guess I'll update it in my application, thanks!

@DimitarTachev
Copy link
Contributor

Hi @NathanaelA,

Thanks for your feedback.

Setting "process": "global.process" is a great idea! In this way, Webpack will not polyfill it with an invalid object and the plugin authors and app developers will be able to override it.

I've opened a pull request with this change.

@NathanaelA
Copy link

@edusperoni - in an app w/o webpack; global.process and process are the exact same thing, literally point to the exact same memory.

But because webpack rewrites all places it sees process with undefined (our/this issue) -- it no longer points to anything because it transforms any code that says "process.blah" be "undefined.blah" which of course breaks anything that used to work...

So what my "hack" does is make webpack rewrite all process with global.process (which is the same object) and so now any code that used to be process.blah will now be transformed to global.process.blah which points to the same object. But because process is still being rewritten; it continues to eliminate the whole issue where webpack attempts to bundle in the node process variable

@DimitarTachev DimitarTachev self-assigned this Jul 3, 2019
@endarova endarova closed this as completed Jul 8, 2019
@darkyelox
Copy link

I'm having problems with this in one of my projects that uses typeorm, when I import and use typeorm somewhere I get the error:

ReferenceError: process is not defined
System.err: 
System.err: StackTrace:
System.err: java.lang.RuntimeException: Unable to create application com.tns.NativeScriptApplication: com.tns.NativeScriptException: Error calling module function 
System.err: ReferenceError: process is not defined
System.err: File: (file: node_modules/supports-color/index.js:5:0)
System.err: 
System.err: StackTrace: 
System.err: ../node_modules/supports-color/index.js(file: node_modules/supports-color/index.js:5:0)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at ../node_modules/chalk/index.js(file: node_modules/chalk/index.js:4:20)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at (file: node_modules/typeorm/browser/platform/PlatformTools.js:1:0)
System.err:     at ../node_modules/typeorm/browser/platform/PlatformTools.js(file:///data/data/org.nativescript.ambulanciasbomberos/files/app/vendor.js:327248:30)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at (file: node_modules/typeorm/browser/driver/sqlserver/SqlServerDriver.js:1:0)
System.err:     at ../node_modules/typeorm/browser/driver/sqlserver/SqlServerDriver.js(file:///data/data/org.nativescript.ambulanciasbomberos/files/app/vendor.js:311455:30)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at ../node_modules/typeorm/browser/migration/MigrationExecutor.js(file: node_modules/typeorm/browser/migration/MigrationExecutor.js:1:0)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at ../node_modules/typeorm/browser/connection/Connection.js(file: node_modules/typeorm/browser/connection/Connection.js:1:0)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at ../node_modules/typeorm/browser/connection/ConnectionManager.js(file: node_modules/typeorm/browser/connection/ConnectionManager.js:1:0)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at ../node_modules/typeorm/browser/index.js(file: node_modules/typeorm/browser/index.js:1:0)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at (file: src/main.ts:2:106)
System.err:     at ./main.ts(file:///data/data/org.nativescript.ambulanciasbomberos/files/app/bundle.js:691:30)
System.err:     at __webpack_require__(file: src/webpack/bootstrap:74:0)
System.err:     at checkDeferredModules(file: src/webpack/bootstrap:43:0)
System.err:     at webpackJsonpCallback(file: src/webpack/bootstrap:30:0)
System.err:     at (file:///data/data/org.nativescript.ambulanciasbomberos/files/app/bundle.js:2:57)
System.err:     at require(:1:266)

@NathanaelA
Copy link

@darkyelox -

  1. Check your webpack config; make sure it still has the line that says "process": "global.process"
  2. Double check what line CLI removes tns_modules in platforms #5 of node_modules/supports-color/index.js:5:0 says

@darkyelox
Copy link

@darkyelox -

  1. Check your webpack config; make sure it still has the line that says "process": "global.process"
  2. Double check what line CLI removes tns_modules in platforms #5 of node_modules/supports-color/index.js:5:0 says

Thank you for your response, my webpack config is the latest and it already has that line.

The line in the index.js file from that library has: const env = process.env; it's a weird issue and dunno what is happening, should I create an issue?

@darkyelox
Copy link

Was my bad, in some of my entities' files I was using import {...} from "typeorm" instead of import {...} from "typeorm/browser", that fix the error, VSCode did that import automatically and I didn't notice it.

@Kripansh
Copy link

@darkyelox can u check again as I am facing the same issue even though I have used import {...} from "typeorm/browser" in all entities.

@darkyelox
Copy link

@darkyelox can u check again as I am facing the same issue even though I have used import {...} from "typeorm/browser" in all entities.

You should use typeorm version 0.2.24 or 0.2.23 as they are the only versions that are working with latest nativescript, assure to use "typeorm": "0.2.23" and not "typeorm": "^0.2.23" in your package.json

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants