fengari icon indicating copy to clipboard operation
fengari copied to clipboard

luaconf.js: ReferenceError: process is not defined

Open silvasur opened this issue 7 years ago • 7 comments

When trying to use src/luaconf.js unmodified in a browser environment, you will get an error like "ReferenceError: process is not defined", since the global process is not defined in a browser environment.

This can easily be fixed by checking typeof process !== "undefined" (as it is already done in other places of fengari):

diff --git a/src/luaconf.js b/src/luaconf.js
index 387e813..422bbfd 100644
--- a/src/luaconf.js
+++ b/src/luaconf.js
@@ -1,6 +1,6 @@
 "use strict";

-const conf = (process.env.FENGARICONF ? JSON.parse(process.env.FENGARICONF) : {});
+const conf = (typeof process !== "undefined" && process.env.FENGARICONF ? JSON.parse(process.env.FENGARICONF) : {});                                                                    

 const {
     LUA_VERSION_MAJOR,

silvasur avatar Jul 14 '18 08:07 silvasur

So this is a bit tricky, we need to be able to pass FENGARICONF at webpack build time. See https://github.com/fengari-lua/fengari-web/blob/815fdcf8d3b03f13f70033f312ea523aae57cda1/webpack.config.js#L37 Adding typeof process !== "undefined" would prevent it from working.

daurnimator avatar Jul 15 '18 02:07 daurnimator

Ah okay, I see. Never mind then.

silvasur avatar Jul 17 '18 20:07 silvasur

Wait no.... I just realised that in webpack we set it to undefined anyway. So we can add it after all.

However that's just for fengari-web's use case. For anyone actually using FENGARICONF for a custom build it would break :(

daurnimator avatar Jul 18 '18 00:07 daurnimator

I'm pretty new to webpack, so this might be a silly idea, but the webpack config file is just JS code, right? That means we could read the FENGARICONF in the config file and build appropriate definitions for the DefinePlugin.

With these patches to fengari and fengari-web, this works just fine:

diff --git a/webpack.config.js b/webpack.config.js
index babb5bc..4d1efd5 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -2,6 +2,20 @@
 
 const webpack = require('webpack');
 
+function fengariconfDefine() {
+	let define = {
+		"process.env.FENGARICONF": "void 0",
+		"typeof process": JSON.stringify("undefined")
+	};
+
+	if (process.env.FENGARICONF) {
+		let fengariconf = JSON.parse(process.env.FENGARICONF);
+		define["__FENGARICONF__"] = JSON.stringify(fengariconf);
+	}
+
+	return new webpack.DefinePlugin(define);
+}
+
 module.exports = [
 	{
 		/*
@@ -33,10 +47,7 @@ module.exports = [
 			]
 		},
 		plugins: [
-			new webpack.DefinePlugin({
-				"process.env.FENGARICONF": "void 0",
-				"typeof process": JSON.stringify("undefined")
-			})
+			fengariconfDefine()
 		]
 	},
 	{
@@ -54,10 +65,7 @@ module.exports = [
 		devtool: 'hidden-source-map',
 		node: false,
 		plugins: [
-			new webpack.DefinePlugin({
-				"process.env.FENGARICONF": "void 0",
-				"typeof process": JSON.stringify("undefined")
-			})
+			fengariconfDefine()
 		]
 	}
 ];

diff --git a/src/luaconf.js b/src/luaconf.js
index 387e813..b1ced5c 100644
--- a/src/luaconf.js
+++ b/src/luaconf.js
@@ -1,6 +1,12 @@
 "use strict";
 
-const conf = (process.env.FENGARICONF ? JSON.parse(process.env.FENGARICONF) : {});
+const conf = (typeof __FENGARICONF__ !== "undefined"
+       ? __FENGARICONF__
+       : (typeof process !== "undefined" && process.env.FENGARICONF
+               ? JSON.parse(process.env.FENGARICONF)
+               : {}
+       )
+);
 
 const {
     LUA_VERSION_MAJOR,

(Admittedly, it's a bit ugly that this basically defines a new global variable __FENGARICONF__, but I think it's okay; a naming conflict with that name seems very unlikely.)

Now you can run FENGARICONF='{"LUA_COMPAT_FLOATSTRING": true}' npm run build and get a fengari-web.js, that prints 0 when running print(0.0). Also, the FENGARICONF environment still works when using fengari-node-cli and you don't get the ReferenceError: process is not defined error any more :)

silvasur avatar Jul 21 '18 07:07 silvasur

(Admittedly, it's a bit ugly that this basically defines a new global variable FENGARICONF, but I think it's okay; a naming conflict with that name seems very unlikely.)

Doesn't that new global result in failure in strict mode? + warnings from linters?

daurnimator avatar Jul 23 '18 00:07 daurnimator

Doesn't that new global result in failure in strict mode? + warnings from linters?

Linters, yes. eslint gives me an no-undef - '__FENGARICONF__' is not defined warning (you could add an /* global __FENGARICONF__ */ at the top of the file to suppress this error).

But since the read access is guarded by typeof __FENGARICONF__ !== "undefined" this will not result in an error.

silvasur avatar Jul 23 '18 06:07 silvasur

But since the read access is guarded by typeof __FENGARICONF__ !== "undefined" this will not result in an error.

When webpack does it's thing, I think it only replaces the uses, not the typeof checks (unless you provide that replacement as well); which means we sort of don't want that check. This is the same issue I pointed at in my first comment.

daurnimator avatar Mar 16 '19 02:03 daurnimator