lblod / ember-rdfa-editor

Ember addon wrapping an RDFa editor with a public API
MIT License
17 stars 5 forks source link

Embroider compatibility #945

Closed Windvis closed 1 year ago

Windvis commented 1 year ago

When trying to build the loket app under Embroider I encountered the following errors related to the editor addon:

1. Unsafe dynamic component > Unsafe dynamic component: this.componentPath in node_modules/.embroider/rewritten-packages/@lblod/ember-rdfa-editor.0c1c90e7/utils/_private/ember-node.js Which comes from this file: https://github.com/lblod/ember-rdfa-editor/blob/8261f1ef8a2b35dd8fc9650832ec4605c68497dd/addon/utils/_private/ember-node.ts#L151-L163 This should be solvable by passing the component class instead of the path and invoking it without the `component` helper: ```js this.template = hbs` {{#unless this.atom}} {{/unless}} `; ``` I'm not sure if this is something used outside of the addon itself, but if it is then I guess it's a breaking change 😄.
2. Ember Velcro component not found [Embroider issue, workaround available] The following error is thrown at build time: > ERROR in ../rewritten-packages/@lblod/ember-rdfa-editor.0c1c90e7/components/toolbar/dropdown.hbs 1:0-58 Module not found: Error: Can't resolve '#embroider_compat/components/velcro' in '/Users/windvis/Development/Projects/digitaal-loket/frontend-digitaal-loket/node_modules/@lblod/ember-rdfa-editor' I briefly looked at it and don't see anything wrong so this is probably an Embroider Bug, I'll try to find out more about this.
3. `crypto` fallback error [DONE] The addon includes some ember-auto-import configuration to provide fallbacks for node-only code. This config isn't used by Embroider, so the build fails. ``` ERROR in ../../@graphy/core.data.factory/main.js 1:15-32 Module not found: Error: Can't resolve 'crypto' in '/Users/windvis/Development/Projects/digitaal-loket/frontend-digitaal-loket/node_modules/@graphy/core.data.factory' BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default. This is no longer the case. Verify if you need this module and configure a polyfill for it. If you want to include a polyfill, you need to: - add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }' - install 'crypto-browserify' If you don't want to include a polyfill, you can use an empty module like this: resolve.fallback: { "crypto": false } ``` It's solvable by adding the same config in the app. I'm not sure if there is a way to handle this from the addon, but I think documenting that it's needed is a good short term solution. I noticed it only seems to complain about the `crypto` module at the moment, so I'm not sure if the other config is still needed?
4. App reexports for components that no longer exist [DONE] This is resolved by #953 Embroider "safe" also complains about 2 components that don't seem to exist (anymore): https://github.com/search?q=repo%3Alblod%2Fember-rdfa-editor%20editor-components&type=code
Windvis commented 1 year ago

Embroider safe also found 2 components that are reexported, but no longer exist. I added it to the description.

Windvis commented 1 year ago

Issue 2 is an Embroider issue, I was able to reproduce it in a test. We can probably work around it by importing the Velcro component and using it as a reference instead:

import { Velcro } from 'ember-velcro';

class SomeComponent {
  Velcro = Velcro;
}
<this.Velcro>...

It's not really a waste since .gjs requires importing component anyways.