ember-cli / ember-cli-htmlbars

MIT License
77 stars 67 forks source link

Runtime error on import { hbs } from 'ember-cli-htmlbars' in storybook #524

Open iamareebjamal opened 4 years ago

iamareebjamal commented 4 years ago

Is ember-cli-htmlbars expected to work in browser context? The storybook example https://github.com/storybookjs/storybook/blob/1f27b252a66e48f4f7106ef7ccc7950a5b715c32/examples/ember-cli/stories/index.stories.js#L1 throws an error at runtime

Cannot read property 'length' of undefined
TypeError: Cannot read property 'length' of undefined
    at includes (http://localhost:6006/vendors~main.925bee91ab6e54312329.bundle.js:113614:31)
    at push../node_modules/resolve-package-path/lib/should-preserve-symlinks.js.module.exports (http://localhost:6006/vendors~main.925bee91ab6e54312329.bundle.js:113622:52)
    at Object.<anonymous> (http://localhost:6006/vendors~main.925bee91ab6e54312329.bundle.js:113404:25)

This happens because ember-cli-htmlbars/utils.js requires hash-for-dep https://github.com/ember-cli/ember-cli-htmlbars/blob/aee846034a6b6772fef7675dad2156e69b40df8e/lib/utils.js#L5

hash-for-dep/module-entry.js requires resolve-package-path https://github.com/stefanpenner/hash-for-dep/blob/f4cc06e526a36f83427b8ce1a6da2ea616112278/lib/module-entry.js#L3

Which accesses process.execArgv which is not defined on webpack's process shim https://github.com/stefanpenner/resolve-package-path/blob/3e9b74512521c69609e84736e64d8e21c52a0c55/lib/should-preserve-symlinks.ts#L17

https://github.com/webpack/node-libs-browser/issues/78

Is this issue exclusive to me? Are others facing this as well? Am I missing some config

Downstream Issue: https://github.com/storybookjs/ember-cli-storybook/issues/35

rwjblue commented 4 years ago

In general, no this package is not intended to be ran in the browser. The issue you are hitting looks a lot like https://github.com/storybookjs/ember-cli-storybook/issues/20 which was fixed (IIRC). Can you double check what versions of storybook and ember-cli-htmlbars thatyou are using?

iamareebjamal commented 4 years ago

storybook 5.3.18 and ember-cli-htmlbars 4.3.1

iamareebjamal commented 4 years ago

I got to the actual problem. It gets transpiled to

var ToStorybook = function ToStorybook() {
  return {
    template: Object(ember_cli_htmlbars__WEBPACK_IMPORTED_MODULE_3__["hbs"])(_templateObject()),
    context: {
      onClick: Object(_storybook_addon_links__WEBPACK_IMPORTED_MODULE_4__["linkTo"])('Button')
    }
  };
};

ember_cli_htmlbars__WEBPACK_IMPORTED_MODULE_3__ has no entry for 'hbs' and hence it turns to Object(undefined) which then throws Object is not a function. Wasn't super easy to debug because of webpack source maps throwing me back at source code again and again. Any idea what might be causing this?

iamareebjamal commented 4 years ago

Interesting. lib/index.js shows no export for hbs

https://github.com/ember-cli/ember-cli-htmlbars/blob/aee846034a6b6772fef7675dad2156e69b40df8e/lib/index.js#L1-L3

Whereas the type files show hbs function exported. And import { hbs } works in ember tests. How is it supposed to work here?

iamareebjamal commented 4 years ago

OK, I see the required config here https://github.com/storybookjs/storybook/commit/583e2679b06f514641b6528e34c97d597a28ff49#diff-bf8aa08fc469c75fe0e5320104847492, but it is still not working. Is there a need for any changes from my end as well?

gossi commented 4 years ago

Every process that uses import { hbs } from 'ember-cli-htmlbars needs to have the babel configuration or else it will not work.

I was working with storybook more intense lately and this design has become a major PITA as it has hit almost everywhere. The major tracks where you may want to use hbs may work, because they are paved but make sure you will not step aside or hell is welcoming you.

@rwjblue can't this be made working without the coupling to that special babel config?

iamareebjamal commented 4 years ago

True that it's very difficult for beginners to debug why it is not working, but still I'm confused what's done differently in the example repo that it works there, and not in my repo. I have tried every permutation and combination now :disappointed:

rwjblue commented 4 years ago

@rwjblue can't this be made working without the coupling to that special babel config?

Not sure how, the templates need to be precompiled before they work (that is what the babel config does). You could ship the template compiler to the app to do the compilation at runtime instead of build time, but that would also require custom setup.

iamareebjamal commented 4 years ago

I think what @gossi is saying that hbs function could be exported without the need of the special babel config like normal node JS exports, and it'll solve most of the problems? Please correct me if I am wrong

rwjblue commented 4 years ago

Ya, I understood the inference, but the issue is that exporting an hbs function that can't run without custom configuration would not be any better than requiring custom babel configuration.

iamareebjamal commented 4 years ago

Hmm. Is there any way I could debug why the config in storybook is not being applied and hbs function is not being exported? Thanks

gossi commented 4 years ago

@rwjblue I do understand this dilemma... and thought about it. My first thought was providing two "exports", one wrapped in babel, the other not... but that would also be stupid. So I had a more purposed approach in thinking this morning. For what I use it is to compile markdown (with ember snytax) into hbs and then put this into:

hbs`${hbs}`

So, I thought the wrapper with babel configured is still a good idea if combined with markdown support on top of it. But as markdown also comes in different flavors and configuration about supported syntaxes, etc. I am lucky to say I have already done a straight-forward config (markdown-it-compiler and markdown-it-ember) for that and I'm using the both very effectively. So, to combine everything (whether ember markdown (emd) or glimmer markdown (gmd) is still up to debate) let's call it gmd for the time being:

import { gmd } from 'somewhere';

const precompiledHbs = gmd`
# Hello

This is markdown with an ember syntax

<MyComponent @foo="bar"/>
`;

And to support the configurationable part, create .gmd.js file in your project root and use the markdown-it-compiler syntax:

module.exports = {
  options: {
    linkify: true,
    html: true,
    typographer: true
  },
  plugins: [
    'markdown-it-abbr',
    'markdown-it-anchor',
    'markdown-it-deflist',
    'markdown-it-highlightjs',
    'markdown-it-ins',
    'markdown-it-mark',
    [
      'markdown-it-plantuml',
      {
        openMarker: '```plantuml\n@startuml',
        closeMarker: '@enduml\n```'
      }
    ],
    'markdown-it-sub',
    'markdown-it-sup'
  ],
  configure(md) {
    // load custom plugins
    md.use(require('./lib/my-md-it-plugin'));
  },
  format(content) {
    if (content.attributes.layout && content.attributes.layout === 'article') {
      return `<Article>${content.html}</Article>`; // <- glimmer component here
    }

    return content.html;
  }
};

I don't know all the low-level parts here, if my idea would fit there. Please rate this.

Also need to summon @pzuraq in here, as we discussed this in storybook discord

rwjblue commented 4 years ago

I think the more appropriate fix here is to remove some of the "magic" from ember-cli-babel configuration, and actually support having a .babelrc.js in the root of the repo. Given that setup most tools that already know about babel itself, will "just work" with the config that we put in there. We can also still solve the constraint of "hiding" that configuration by exporting a shared function that you invoke from ember-cli-babel in that config file.

I'll have to think on it a bit more, but we are almost certainly going to have to do this anyways (for Babel 8 support).

gabrielcsapo commented 3 years ago

This should be a part of the preset https://github.com/storybookjs/storybook/blob/7064642e1aee7786c77fe735c064c0c29dbcee01/app/ember/src/server/framework-preset-babel-ember.ts can you see if this works without any changes?