bustle / ember-mobiledoc-editor

MIT License
86 stars 48 forks source link

Mobiledoc renderer included in vendor.js twice #156

Open sdhull opened 6 years ago

sdhull commented 6 years ago

After building, try:

grep "define('mobiledoc-dom-renderer\/utils\/tag-names'" dist/assets/vendor.js

You will see 2 matching lines.

If you include ember-mobiledoc-dom-renderer in your app as well, try:

grep "define('ember-mobiledoc-dom-renderer/mobiledoc-dom-renderer\/utils\/tag-names'" dist/assets/vendor.js

You will see 1 matching line.

That means that if you want to edit & render mobiledoc with an ember app, you will include mobiledoc-dom-renderer in vendor.js 3 times. Aside from being just plain crazy, it's a problem for those of us concerned about page weight.

IMO this addon should not require the dom renderer at all (let alone twice). It's fine to have it in dev dependencies (for tests), but afaict, the addon code itself does not use the dom renderer anywhere. So it shouldn't be in dependencies and it shouldn't be imported in index.js either.

Also mobiledoc-kit adds mobiledoc-dom-renderer to dependencies as well, but afaict it's just to support editor.serialize('html'), even though it has a fallback way to generate that html. IMO, none of the editor packages ought to require the renderer packages (since the renderers are not used to generate the html for the editors).

lukemelia commented 4 years ago

I've confirmed that this is still a problem today.