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

luaconf.js: ReferenceError: process is not defined #129

Open
silvasur opened this issue Jul 14, 2018 · 7 comments
Open

luaconf.js: ReferenceError: process is not defined #129

silvasur opened this issue Jul 14, 2018 · 7 comments

Comments

@silvasur
Copy link

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,
@daurnimator
Copy link
Member

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.

@silvasur
Copy link
Author

Ah okay, I see. Never mind then.

@daurnimator
Copy link
Member

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 :(

@silvasur
Copy link
Author

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 :)

@daurnimator
Copy link
Member

(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?

@silvasur
Copy link
Author

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.

@daurnimator daurnimator reopened this Dec 1, 2018
@daurnimator
Copy link
Member

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.

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

No branches or pull requests

2 participants