handsontable / vue-handsontable-official

Vue Data Grid with Spreadsheet Look & Feel. Official Vue wrapper for Handsontable.
https://handsontable.com/docs/vue
MIT License
746 stars 128 forks source link

Fix a problem with tree shaking the es build. #195

Closed jansiegel closed 4 years ago

jansiegel commented 4 years ago

Context

The issue seems to be caused by an update in the @vue/component-compiler package, specifically the update from 4.2.0 to 4.2.1, and even more specifically, this PR: https://github.com/vuejs/vue-component-compiler/pull/96. It's adding a /*#__PURE__*/ comment to the component being the default export of the package (in our case, it's HotTable).

var __vue_script__ = HotTable;

// (...)

var __vue_component__ = /*#__PURE__*/normalizeComponent({
  render: __vue_render__,
  staticRenderFns: __vue_staticRenderFns__
}, __vue_inject_styles__, __vue_script__, __vue_scope_id__, __vue_is_functional_template__, __vue_module_identifier__, false, undefined, undefined, undefined);

Unfortunately, that caused the following issue:

In the es build (and commonjs as well ), we're exporting the library like this:

export default __vue_component__;
export { BaseEditorComponent, HotColumn, HotTable };

(The __vue_component__ is the HotTable object after transformation from the normalizeComponent function.)

The problem emerges when we're not using the default export when importing HotTable. Then, the entire normalizeComponent logic get's tree-shaken, and we're left with the raw HotTable object (imported from a named export), which doesn't work properly (it doesn't contain the template renderers etc).

My fix is a small rollup plugin, which takes the default export and puts it in the place of a named HotTable export, so we're always using the "normalized" component, regardless of the import method.

I could not reproduce this issue for the commonjs build, but I figured it could potentially happen, so I extended the plugin to work for it as well.

(Another approach to fixing this problem would probably be building each component (HotTable, HotColumn and BaseEditorComponent separately, each with a default export, and bundling them after that, but my current concept was quicker)

How has this been tested?

It's hard to write a test case for this problem, as it happens when tree shaking the es build, so I just tested it manually. @wojciechczerniak, @aninde I would HIGHLY suggest testing the production and development builds of an external app using the wrapper to our standard testing checklist.

Types of changes

Related issue(s):

  1. 188

Checklist:

codesandbox-ci[bot] commented 4 years ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 272fba4cf206f37b893638219ce6ca3e4084f138:

Sandbox Source
handsontable/examples: pull-request Configuration
jansiegel commented 4 years ago

My fix is a small rollup plugin, which takes the default export and puts it in the place of a named HotTable export, so we're always using the "normalized" component, regardless of the import method. (...) (Another approach to fixing this problem would probably be building each component (HotTable, HotColumn and BaseEditorComponent separately, each with a default export, and bundling them after that, but my current concept was quicker)

Thanks to @wojciechczerniak, we have a much simpler fix, which is a different approach to reexporting the components (23a35a3).