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

Unexpected token: punc (.) #271

Closed
lid3rs opened this issue Nov 7, 2020 · 51 comments · Fixed by #360
Closed

Unexpected token: punc (.) #271

lid3rs opened this issue Nov 7, 2020 · 51 comments · Fixed by #360
Labels

Comments

@lid3rs
Copy link

lid3rs commented Nov 7, 2020

Version: 5.1.0
Node: 15.0.1
Webpack: 5.4.0

In webpack:

  new Dotenv({
      path: path.resolve(__dirname, '../../config', '.env.production'),
    }),

This causes error somehow. Error:

ERROR in server.bundle.js from Terser
Unexpected token: punc (.) [server.bundle.js:3367,6]
@mrsteele
Copy link
Owner

Could it be an error in webpack? Have you tried falling back to webpack 4? Just curious because I don’t see much code interaction with this plugin.

@lid3rs
Copy link
Author

lid3rs commented Nov 11, 2020

Could it be an error in webpack? Have you tried falling back to webpack 4? Just curious because I don’t see much code interaction with this plugin.

Unfortunately not.

@mrsteele
Copy link
Owner

Can anyone give me an example repo? This just doesn't help me reproduce it on my end

@lid3rs
Copy link
Author

lid3rs commented Nov 11, 2020

Can anyone give me an example repo? This just doesn't help me reproduce it on my end

I went very from it, but I will review the code and if this appears again, will create a repo.

@sapkra
Copy link

sapkra commented Nov 20, 2020

I'm getting the same error but with a little bit more details (it only seems to appear when building for node environments):

App threw an error during load
/Users/sapkra/Documents/git/lyno/packages/client-electron/build/main.js:5187
                {}.DEBUG = namespaces;
                  ^

SyntaxError: Unexpected token '.'
    at wrapSafe (internal/modules/cjs/loader.js:1060:16)
    at Module._compile (internal/modules/cjs/loader.js:1108:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1173:10)
    at Module.load (internal/modules/cjs/loader.js:992:32)
    at Module._load (internal/modules/cjs/loader.js:885:14)
    at Function.f._load (electron/js2c/asar_bundle.js:5:12694)
    at loadApplicationPackage (/Users/sapkra/Documents/git/lyno/node_modules/electron/dist/Electron.app/Contents/Resources/default_app.asar/main.js:110:16)
    at Object.<anonymous> (/Users/sapkra/Documents/git/lyno/node_modules/electron/dist/Electron.app/Contents/Resources/default_app.asar/main.js:222:9)
    at Module._compile (internal/modules/cjs/loader.js:1152:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1173:10)

within this code in my bundle:

/**
 * Save `namespaces`.
 *
 * @param {String} namespaces
 * @api private
 */
function save(namespaces) {
	if (namespaces) {
		{}.DEBUG = namespaces;
	} else {
		// If you set a process.env field to null or undefined, it gets cast to the
		// string 'null' or 'undefined'. Just delete instead.
		delete {}.DEBUG;
	}
}

I read something like that in the background the process.env variable will be injected which is {} in that case.
The related change seems to be: 3ecc1e0

@martinjuhasz
Copy link

I'm also seeing this error using the latest dotenv-webpack 5.1.0 and the latest electron-forge which seems to use webpack 4.44.2. Everything works fine with dotenv-webpack 4.0.0.

A JavaScript error occurred in the main process
Uncaught Exception:
/home/.../desktop/.webpack/main/index.js:671
    {}.DEBUG = namespaces;
      ^

SyntaxError: Unexpected token '.'
    at wrapSafe (internal/modules/cjs/loader.js:1060:16)
    at Module._compile (internal/modules/cjs/loader.js:1108:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1173:10)
    at Module.load (internal/modules/cjs/loader.js:992:32)
    at Module._load (internal/modules/cjs/loader.js:885:14)
    at Function.f._load (electron/js2c/asar_bundle.js:5:12694)
    at loadApplicationPackage (/home/.../desktop/node_modules/electron/dist/resources/default_app.asar/main.js:110:16)
    at Object.<anonymous> (/home/.../desktop/node_modules/electron/dist/resources/default_app.asar/main.js:222:9)
    at Module._compile (internal/modules/cjs/loader.js:1152:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1173:10)

@mrsteele
Copy link
Owner

First, I think one of the problems is that you might be referencing a variable that does not exist. The "last-ditch" effort of this plugin is to make your code still run, because if you have an optional variable, say VERBOSE and you have something like:

if (process.env.VERBOSE) {
  // talk about it
}

then if you DON'T set it your app should do something like:

if ({}.VERBOSE) { // which equates to "falsey" because it will resolve to "undefined"
  // talk about it
}

I wonder how you are calling the script, I just wrote this to show its a totally valid way of working:
Screenshot 2020-11-25 071825

Looking at the code sample called out @martinjuhasz I can see something is being set in the env like so:

// looks like you have:
process.env.DEBUG = namespaces

obviously that would cause some problems:
Screenshot 2020-11-25 072905

If we remove the process.env = {} definition, it would solve your problem, but also break anyone else's code that uses optional definitions... I'm glad I know the problem know just not sure what we can do to make it not mess up other things... Perhaps we can just make sure we load it at the end (not sure, haven't tested this but would love to see if that magically fixes anyone)

@sapkra to your point:

I read something like that in the background the process.env variable will be injected which is {} in that case.

I saw that in webpack 4, but it was changed in webpack 5, so errors were being throwin because `"process" is not defined" in the browser where undefined variables existed.

Happy to hear some thoughts, also interested if anyone has an example repo I can take a look at.

@mrsteele
Copy link
Owner

Just double-checked this in my dotenv-webpack-example repo and I'm right about removing that line and causing unreferenced process.env variables to throw errors:

Screenshot 2020-11-25 074312

@martinjuhasz
Copy link

my error occurs in the exact same location as @sapkra , as i'm also using electron. This code is not from us, but is generated somehow from a dependency. I was not able to find out from which one tho.

/**
 * Save `namespaces`.
 *
 * @param {String} namespaces
 * @api private
 */
function save(namespaces) {
	if (namespaces) {
		{}.DEBUG = namespaces;
	} else {
		// If you set a process.env field to null or undefined, it gets cast to the
		// string 'null' or 'undefined'. Just delete instead.
		delete {}.DEBUG;
	}
}

@thisVioletHydra
Copy link

Just double-checked this in my dotenv-webpack-example repo and I'm right about removing that line and causing unreferenced process.env variables to throw errors:

Screenshot 2020-11-25 074312

process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';

How can i use this setting with new webpack 5? Everything worked with the old one, and the node did not swear at the lack of a certificate during development, and now I cannot even influence this parameter

@sapkra
Copy link

sapkra commented Nov 26, 2020

@martinjuhasz I had a look in my dependencies and searched for this code. It's part of a file called node.js in a package called debug. I this error will only happen when webpack is trying to built something for node.

https://github.com/visionmedia/debug/blob/master/src/node.js#L205

In electron the debug package is a dependency of electron-download and electron-squirrel-startup in my case.

@thisVioletHydra You could try to set the env var in your environment. You can do this in your package.json script too.

@mrsteele
Copy link
Owner

For kicks, you are welcome to test commenting out this line in node_modules/dotenv-webpack/dist/index.js:

https://github.com/mrsteele/dotenv-webpack/blob/master/src/index.js#L148

I'm considering making it optional, as it seems like this could become a huge problem if webpack does this... Not sure why this was working before and now posing such a challenge...

@sapkra
Copy link

sapkra commented Nov 26, 2020

@mrsteele Just to point it out again: This issue is reported for webpack 4 and 5.

@martinjuhasz
Copy link

@sapkra is there a way to resolve this for an bootstraped electron application for now?

@mrsteele
Copy link
Owner

Curious on your thoughts about this: #289

Essentially removes stubs, but makes the developer responsible for ALYWAYS referencing the variables. A little more strict but I think this would really help what you all may be running into.

If you are able to test it out, let me know. Essentially does what I suggested here but in your declaration configs.

@zohu
Copy link

zohu commented Dec 23, 2020

[email protected] avoids this problem, and Webpack does not replace process.env

@mrsteele
Copy link
Owner

mrsteele commented Jan 4, 2021

Anyone get a chance to check out my PR?

#289

This would allow you to remove the "polyfill", which means everything needs to be explicitly defined, but also allows this situation too be resolved...

@sibelius
Copy link

sibelius commented Jan 8, 2021

process.env is broke on server on lates version (v6) webpack 4

serverless-nextjs/serverless-next.js#866

can we have tests on both web and server to avoid regressions?

we also need to test against webpack 4 and webpack 5 now

@mrsteele
Copy link
Owner

mrsteele commented Jan 8, 2021

@sibelius you see #289?

The challenge is you cannot have "empty" values, while also having the ability to set process.env.whatever = "okay"

@sibelius
Copy link

sibelius commented Jan 8, 2021

maybe we should avoid using dotenv-webpack in production for server builds

adding a note on readme, and also a warning when using the plugin with mode: production and target: node

@sibelius
Copy link

sibelius commented Jan 8, 2021

version 4.0.0 works well

I think we should only replace process.env.{} that are available at the build time

and each the rest like process.env.<env-name>

@renatoalencar
Copy link

The problem seems in this library: https://github.com/visionmedia/debug/tree/master/src

When it tries to check if is a browser environment it probably fails due to this line https://github.com/visionmedia/debug/blob/master/src/index.js#L6

Defining somethine like process.browser to true using Webpack DefinePlugin should probably work.

@mrsteele
Copy link
Owner

mrsteele commented Jan 8, 2021

If thats your problem, I think you need to update your webpack config targets right?

@lid3rs
Copy link
Author

lid3rs commented Feb 24, 2021

image

Confirming for build for node.

@lid3rs
Copy link
Author

lid3rs commented Feb 24, 2021

maybe we should avoid using dotenv-webpack in production for server builds

adding a note on readme, and also a warning when using the plugin with mode: production and target: node

How you avoided it? Can you copy-paste example of config?

@lid3rs
Copy link
Author

lid3rs commented Feb 27, 2021

I think i moved a bit further for now. I removed dotenv-webpack module, and did:

const dotenv = require('dotenv').config({
  path: path.resolve(__dirname, '../../config', '.env.production'),
});

Later:

    new webpack.DefinePlugin({
      'process.env': JSON.stringify(dotenv.parsed),
    }),

After build I got this error (I simplified the function):

SyntaxError: Unexpected token ':'

Screenshot 2021-02-27 at 21 12 05

Then I was thinking, that this gives no error:

function save(namespaces) {
  if (true) {
  }else{
    return {"jucsjh":"ehjd"}.DEBUG = namespaces;
  }
}

Screenshot 2021-02-27 at 21 13 05

Then I thought, maybe everywhere where I see similar problem I will continue putting returns, and problem is actually not in the symbol...

And yes, all worked. So the problem at least in this example is in 'return' statement after build.
I will try to use again dotenv-webpack. Will see if the same approach can be applied there as well.

@lid3rs
Copy link
Author

lid3rs commented Feb 27, 2021

Screenshot 2021-02-27 at 21 22 34
Screenshot 2021-02-27 at 21 22 24

@sapkra
Copy link

sapkra commented Feb 27, 2021

First of all {}.DEBUG = namespaces doesn't make any sense at all because the value can never be read again.
And second, env vars should never be set like this: process.env.DEBUG = namespace. In my opinion, it has to be refactored inside the debug library.

@lid3rs
Copy link
Author

lid3rs commented Feb 27, 2021

But then this bug should be in debug git, not here, right?

@sapkra
Copy link

sapkra commented Feb 27, 2021

@lid3rs I have created an issue. It's linked above.

@lid3rs
Copy link
Author

lid3rs commented Feb 27, 2021

Thanks! Then let's redirect our activity there, as this repair is really needed :)

@lid3rs
Copy link
Author

lid3rs commented Feb 28, 2021

upd, today I moved back to "dotenv-webpack" from

const dotenv = require('dotenv').config({
  path: path.resolve(__dirname, '../../config', '.env.production'),
});

I used above, applied the same "hack" with 'return' and it worked nicely. So confirming, it's not the "dotenv-webpack"

@sapkra
Copy link

sapkra commented Feb 28, 2021

Yeah, but this will break the debug library because the webpack.DefinePlugin replaces process.env with {} even when it's a setter.

dotenv-webpack should fix it until it will be changed in debug. But of course no one can guarantee that other libraries will not introduce a setter for env vars as well. It's more a conceptional problem of how the DefinePlugin works.

@mrsteele
Copy link
Owner

I think this brings up a fundamental issue with how multiple libraries are treating your environment variables.

For the context of this plugin (dot env-webpack), it is used to expose your variables to the client.

The "debug" plugin looks to be writing to your environment.

Due to security, this plugin will only expose what you explicitly reference. That being said if your code references something that doesn't exist, the results are empty:

if ({}.DEBUG) {
  // this compiles
}

It looks like "debug" has an interesting example of trying to write to the environment at runtime, which doesn't work with this plugin and probably should break on the client as well.

{}.DEBUG = true
// obviously doesn't compile

I wrote a PR to possible not override empty variables, but looking at it more it won't really solve the problem as the client would start spitting out "cannot read 'env' of undefined" when trying to set process.env.DEBUG = true.

Webpack used to polyfill process for you, but as of v5 they removed it. Maybe this could all be resolved for you by adding in the polyfill again?

@sapkra
Copy link

sapkra commented Feb 28, 2021

So then dotenv-webpack should remove the current solution to override process.env and add the info to the docs to polyfill process.env in browser environments. Because when you are building a node application with webpack the polyfill is not necessary I think.

@mrsteele
Copy link
Owner

mrsteele commented Mar 1, 2021

@sapkra Disagree with you on removing the override, as that solves issues with non-referenced env variables where applicable. While node environments have context to process.env, browsers dont. In fact I would love for you to take a look at a PR I already opened to address this very thing: #289.

I would love to hear from others if the webpack configuration adjustments resolve this issue. this article seems to be a good starting point for anyone who currently has a broken/incompatible plugin.

@sapkra
Copy link

sapkra commented Mar 1, 2021

Wouldn't it be possible to detect if the webpack build target is for node or for browser environments and decide dependend on it if the plugin should override process.env?

@beeequeue
Copy link
Contributor

@mrsteele The reason this started popping up is that for some reason specifically the {}.NAME stub stopped working. We had it in our config as well, then all of the sudden had to change it due to getting this error.

The problem seems to be specifically the {}. part, and changing it to be something like "ENV_VAR_NOT_FOUND". instead will make it work again!

I can attest that this is the current problem as right now we are using dotenv in dev mode which is breaking due to this error, but when we build for production we use our custom stub which works and does not error.

The exact config we're using right now:

webpack.DefinePlugin({
  "process.env": '"ENV_VAR_NOT_FOUND"',
})

@beeequeue
Copy link
Contributor

beeequeue commented Mar 3, 2021

Wouldn't it be possible to detect if the webpack build target is for node or for browser environments and decide dependend on it if the plugin should override process.env?

This should be possible with the target webpack option

@mrsteele
Copy link
Owner

mrsteele commented Mar 3, 2021

I would love to spend some time to resolve this immediately because of the challenge its posing, unfortunately things are busy at work and home.

I looked briefly for ways to look at the webpack config within the context of my plugin and I didn't see anything obvious (though it should be there right?) If anyone has an idea/suggestion I'd recommend trying it out and submitting a PR.

Otherwise, it might be a matter of removing from the code and writing something in the README to explain how to polyfill for the browser when the ENV is empty and not breaking on the browser. If anyone could volunteer to help with that it would be greatly appreciated.

This has been on my thoughts the past few days, just can't get enough time unfortunately to dedicate to it at the moment.

@beeequeue
Copy link
Contributor

I'm currently making a PR for just using the "MISSING_ENV_VAR" replacement, as well as not stubbing it at all if the target is node, but I'm struggling find a way to get the webpack config for that part.

Then again it might be a good idea to just create another breaking change and not stub it at all...

As long as there's proper documentation for it here no one should come here to complain about it, right? 😄

@mrsteele
Copy link
Owner

mrsteele commented Mar 4, 2021

anyone else want to weigh in on #360?

  • Either we use webpack config to determine when to stub
  • Or we configure it when initializing

If no one else chimes in we will just move forward as is, a little uneasy with it automatically interpreted but could just be my nerves with a big change like this...

@github-actions
Copy link

github-actions bot commented Mar 5, 2021

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mrsteele
Copy link
Owner

mrsteele commented Mar 5, 2021

This was automatically closed as part of the CI, but would love any confirmation that things look resolved with the updated version fixes thanks to @beeequeue

@beeequeue
Copy link
Contributor

I can confirm that it fixed our runtime error

@thisVioletHydra
Copy link

working like a charm !

@moroine
Copy link

moroine commented Apr 26, 2021

I'm using a library which is relying on process.env[name] dynamically. So replacing with MISSING_ENV_VAR is not working.

I had to remove dotenv-plugin for @lid3rs solution adapted to bypass the syntax error:

const { config: configDotEnv } = require('dotenv');
const { DefinePlugin, ProvidePlugin } = require('webpack');
const dotenv = configDotEnv();
new ProvidePlugin({ process: 'process' }),
new DefinePlugin({
  'process.env': `(${JSON.stringify(dotenv.parsed)})`,
}),

@mrsteele do you think we could do this in dotenv-plugin instead?

@sapkra
Copy link

sapkra commented Apr 26, 2021

@moroine It would definitely increase the build size. Which library are you using?

@moroine
Copy link

moroine commented May 4, 2021

yeah, it increases the build size but this could be opt-in. It's a custom library that is just doing:

export function getEnv(name: string): string {
  const value = process.env[name];

  if (value == null || value.lenght === 0) {
    throw new Error(`envService.getEnv: missing ${name} env variable`);
  }

  return value;
}

export function getOptionalEnv(name: string): string | null {
  const value = process.env[name];

  if (value == null || value.lenght === 0) {
    return null;
  }

  return value;
}

Because if we're not including system env variable, but just the one from .env then it would not increase the bundle size as all values would be shipped anyway.

@mrsteele
Copy link
Owner

mrsteele commented May 4, 2021

@moroine a few things:

  1. This is not on topic with the issue, and this issue has closed. Please open a new ticket if you would like to discuss.
  2. If we kept all the envs, we are not really bundling envs anymore but stashing all of them which does not meet one of the plugin goals (Only bundle what you use for security purposes and bundle sizes like @sapkra mentions)

@sapkra
Copy link

sapkra commented May 4, 2021

@moroine It's not that pretty but you could just refactor your code like this:

export function requiredEnv(name: string, value: string): string {
  if (!value) {
    throw new Error(`envService.getEnv: missing ${name} env variable`);
  }

  return value;
}

requiredEnv('MY_ENV', process.env.MY_ENV);

But apart from that it's not really smart to have runtime checks for env vars all over the code. It would be better to have one central point on initialization of your app to check if your vars are set properly and add types for process.env.

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

Successfully merging a pull request may close this issue.

10 participants