global is not defined in each.js

[dennisblack666]

After the version 4.1.2 there is an error in the each.js helper file.

ReferenceError: global is not defined

[nknapp]

That's strange because there was no problem with the build and the tests.

Who is showing this error measure? You IDE? Eslint?

[dennisblack666]

The bug is in the Google Chrome Console when the context is an object.

When i remove the "global." property it works correct (jumps into the else statement) and the template shows the correct values, without the ReferenceError.

[nknapp]

Could you describe exactly, what you are doing? Which file are you looking at (which URL)? In the built handlebar.js file, the each helper is wrapped with a function by Webpack, which defines "global" as a parameter". I cannot reproduce your issue anywhere.

[dennisblack666]

Found the issue in our webpack.config. Changed the property "node: false" to "node: {global: true}".

Thank you for your help!

[kjg]

I'm having this same issue. The specs define global

handlebars.js/spec/env/common.js

Line 1 in 0c8230c

var global = (function() {

which is why this doesn't cause any issues in the tests.

Is it expected that we should have to always define global if we use handlebars?

[nknapp]

OK, the change was made when #1557 was merged. So, this is a regression. As you say, there wasn't a test to catch this problem and I hadn't seen this coming when I merged the PR.

If you have a suggestion on how to solve this and you are willing to open a PR, go ahead.

I think there should be 3 steps in a PR:

1. Have a test. Since the setup for unit-tests is pretty fixed, and this issue depends on webpack configuration, it should be an integration-test. The folder integration-testing already contains some tests that test handlebars with different webpack and babel configs. Maybe you could add a new one that reproduces this problem.

2. Implement a solution. Do you have an idea how to solve this? I believe it could be done by checking typeof Symbol instead of global.Symbol, but I am not sure.

3. Add an eslint-rule: I don't know if there is an eslint-plugin that would prevent such an error in the future. But if there is, we should integrate it.

[kjg]

Thanks so much for reopening this. Let me see if I can allocate some time to work on a PR, however I'm not 100% sure I'll have that leeway. I'll have to dig in deeper to figure out exactly what the check is doing, but if it's just to determine if Symbol is currently defined in scope, then I would assume checking typeof Symbol would work without the global.

If anyone else needs a workaround in the meantime this workaround is working for me:

My use case is a webpack 4 config with target: 'electron-renderer' (I don't have the node property set, so I'm assuming it's using whatever default it might set for electron-renderer target.

By adding new webpack.DefinePlugin({global: 'window'}) to my webpack plugins config. I was able to not trigger this error.

[kjg]

After about a day of trying to figure out a good way to test this, I'm not really sure how to proceed.

The way to trigger this issue appears to be: precompile a template and include the CJS handlebars runtime via webpack, then run the resulting JS code in a browser where global is not defined and the CJS handlebars runtime doesn't define it.

I can't figure out a great way to write an integration test that requires webpack compile the code then run the js via selenium or some such.

Any ideas?

[nknapp]

@kjg You sounded like you already have a setup that reproduces the error. I thought you could use that to derive a minimal project (package.json, webpack.config.js, entrypoint etc) from this setup and add this project to the integration tests. If you share your config, I might be able to help.

[kjg]

Here is a branch with a minimal test case. Once built, the html file needs to be opened in a browser to see the error. I don't have a way to get this to actually fail the CI build yet since it requires running in a browser.

4.x...kjg:handlebars-webpack-electron

[nknapp]

Maybe jsdom will do the trick... Probably won't because "global" is also present.

[nknapp]

Maybe #1688 is the solution to address this in a test-case. Since the tests already run in Saucelabs, it should fail in the browsers. Unfortunately, the Internet-Explorere Saucelabs tests are not working at the moment, so the tests will fail anyway.

[kjg]

I spend a long time trying to get this to fail in the browser tests.

Removing global from spec/env/common.js is a good step. However from what I've found all the browser tests rely on dist/handlebars.js or dist/handlebars.runtime.js

The way these files are built via webpack seems to handle defining global as set to window. It's only when using dist/cjs/handlebars.runtime.js as the source (such as when using webpack in our own project to bundle the runtime (like via handlebars-loader) that the issue arises. Also I think it might be specific to the webpack config target of electron-renderer. I couldn't reproduce with target of web

Maybe it could be argued that this is a bug in the way webpack treats a target of electron-renderer?

This certainly seems to be a pretty narrow edge case.

However since handlebars doesn't make extensive use of 'global' other than this one check, maybe it would still be okay to for us to fix it here?

[nknapp]

It might also be enough to forbid the global "global" in the linting rules.

[jaylinski]

I opened a PR to fix this issue: #1692

Would be nice if someone could take a look at it.

[lwr]

we have this problem and now using this workarounds (ensuring Symbol is correctly polyfilled)

new webpack.DefinePlugin({
    global          : undefined,
    'global.Symbol' : 'Symbol', // workarounds https://github.com/handlebars-lang/handlebars.js/issues/1593
}),

[steveoh]

the vitejs solution is

import { defineConfig } from 'vite';

export default defineConfig({
  define: {
    global: {},
  },
});

[aleen42]

I opened a PR to fix this issue: #1692

Would be nice if someone could take a look at it.

@nknapp is there a plan to push on this PR for solving this issue?

[eltonfms]

@jaylinski is there any prediction on when these new features will be available?
PR 1776

Unfortunately, Vite is not interested in providing a solution:
2618, 7257, 5912, and 12163.

[jaylinski]

I released a new patch version (v4.7.8) on npm. Sorry it took so long.