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

Missing helpers after latest update #1553

Closed
paulfalgout opened this issue Sep 18, 2019 · 6 comments

Comments

@paulfalgout
Copy link
Contributor

commented Sep 18, 2019

Before filing issues, please check the following points first:

This will probably help you to get a solution faster.
For bugs, it would be great to have a PR with a failing test-case.

I can try and find a way to reproduce this, but to get it started in case it's somehow obvious. In 4.1.2 everything with the below helpers worked fine. In 4.2.0 I get "Missing helper: "far""... but it only seems to error when I'm instrumenting the code with istanbul.

However there are no other changes npm i handlebars@4.1.2 fixes the issue and @4.2.0 causes it again.

import _ from 'underscore';
import Handlebars from 'handlebars/runtime';
import { findIconDefinition, icon, config } from '@fortawesome/fontawesome-svg-core';
import '@fortawesome/fontawesome-svg-core/styles.css';

config.autoAddCss = false;
config.replacementClass = 'svg-inline--fa icon';

function faHelper(prefix, iconName, { hash = {} }) {
  const faDefinition = findIconDefinition({ prefix, iconName });
  const faIcon = icon(faDefinition, hash);

  if (!faIcon || !faIcon.html) {
    throw new Error(`${ prefix }:${ iconName } fontawesome icon not loaded`);
  }
  return new Handlebars.SafeString(faIcon.html);
}

// {{far "acorn"}} -> <svg ...>
Handlebars.registerHelper({
  far: _.partial(faHelper, 'far'),
  fas: _.partial(faHelper, 'fas'),
  fal: _.partial(faHelper, 'fal'),
});
@paulfalgout paulfalgout referenced this issue Sep 18, 2019
1 of 1 task complete
@nknapp

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2019

There are a couple of things that I would like you to try:

  • Go to your node_modules/handlebars and remove the browser property from the package.json, see if it works then.
  • Replace the browser tag by
  ...
  "browser": {
    ".": "./dist/cjs/handlebars.js",
    "./runtime": "./dist/cjs/handlebars.runtime.js"
  },
 ....

and see if it works then. If it doesn't then...

then I need additional data to analyze the issue myself.

  • A package.json
  • A webpack.config.js
  • The code that actually compiles and executes the template (the project you have back-referenced, uses the babel-plugin-handlebars-inline-precompile. Is that the part where the error is thrown?)
nknapp added a commit that referenced this issue Sep 18, 2019
related to #1553

- registering helpers on an instance retrieved via
  `import`, compiling the template on an instance
   retrieved via `require`
- using `@roundingwellos/babel-plugin-handlebars-inline-precompile` to load plugins inline
@nknapp

This comment has been minimized.

Copy link
Collaborator

commented Sep 18, 2019

I did some testing, with babel-plugin-handlebars-inline-precompile

This one seems to work:

import * as Handlebars from 'handlebars/runtime'
import hbs from 'handlebars-inline-precompile';

Handlebars.registerHelper('loud', function(text) {
    return text.toUpperCase();
});

const template = hbs`{{loud name}}`;
const output = template({name: 'yehuda'})

This one results in the error you decribe:

import * as Handlebars from 'handlebars'
import hbs from 'handlebars-inline-precompile';

Handlebars.registerHelper('loud', function(text) {
    return text.toUpperCase();
});

const template = hbs`{{loud name}}`;
const output = template({name: 'yehuda'})

The problem with the second example is that the babel-plugin loads handlebars/runtime and we register the helper at handlebars.

Could you check that there is only one version of Handlebars in your dependency tree. There might be a 4.1.2 and a 4.2.0. If registerHelper is called on the 4.2.0-runtime and the babel-plugin uses 4.1.2 to compile the template, you would get exactly the error Missing helper.

@paulfalgout

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

Thanks for looking into this.. I'll dig in shortly and tell you what I find

@paulfalgout

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

Ok well it's definitely something with multiple runtimes, but I think it's happening somewhere in webpack. I'm only getting a single handlebars dependency in node_modules, but both the .min.js and the regular source are getting compiled.
4.2.0:
Screen Shot 2019-09-19 at 2 33 28 PM
4.1.2:
Screen Shot 2019-09-19 at 2 36 11 PM

Next I'll look at your first suggestion and modify the browser settings, which I'm expecting will fix this. I'll see if I can track down under what circumstances it's using which one.

@paulfalgout

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

Removing the browser from the node_modules package.json as originally suggested solves the problem for v4.2.0

I don't know that the browser package.json config is desired. In the very least I do think it'll be breaking for some so should be a v5, but even still...

This ensures that if you import Handlebars from 'handlebars/runtime'; then you cannot use any other import of Handlebars in the same project as you'll be importing either the other compiled full version including it's own runtime or you'll be importing the other modules. Since browser is what webpack defaults to pulling in, it seems unlikely that a pre-minified package is the best option there. Even if a built file is the one, it should not be minified as the project bundler will want to decide that, and the spec that webpack goes by defines the browser field as the script that the bundler will server to the browser: https://github.com/defunctzombie/package-browser-field-spec

Actually after testing and looking at related issues with the angular build that prompted this change #1174 (comment) I believe the correct setting for browser is:

  "browser": {
    ".": "./dist/cjs/handlebars.js",
    "./runtime": "./dist/cjs/handlebars.runtime.js"
  },

While it seems odd that we'd be giving the browser cjs builds, browser is somewhat of a misnomer as this is what we're feeding to browser bundlers. While some users may have been importing built files for angular which solved their problem, it was a wrong solution. They should have been using the cjs builds.

paulfalgout added a commit to paulfalgout/handlebars.js that referenced this issue Sep 19, 2019
We should not be using pre-built files for the `browser`.  Resolves wycats#1553
paulfalgout added a commit to paulfalgout/handlebars.js that referenced this issue Sep 19, 2019
We should not be using pre-built files for the `browser`.  Resolves wycats#1553
nknapp added a commit that referenced this issue Sep 20, 2019
We should not be using pre-built files for the `browser`.  Resolves #1553
nknapp added a commit that referenced this issue Sep 20, 2019
related to #1553

- registering helpers on an instance retrieved via
  `import`, compiling the template on an instance
   retrieved via `require`
- using `@roundingwellos/babel-plugin-handlebars-inline-precompile` to load plugins inline
@nknapp nknapp closed this Sep 20, 2019
@nknapp

This comment has been minimized.

Copy link
Collaborator

commented Sep 20, 2019

@paulfalgout It would still be great to know the exact cause of the problem, so we can add an integration test that tests this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.