ember-svg-jar
ember-svg-jar copied to clipboard
just assetResolve interface
@jherdman here's only the interface PR we talked about at #189
A few things to notice:
- It guards against fastboot, not sure how dynamic imports work in fastboot (guidance welcome)
- If we are going to write tests, we will need to just don't run them for embroider, at least for
hbs
strategy at this moment, not sure how to have some sort of "allowlist". For them to work for embroider builds, a whole new strategy with webpack loaders probably should be used instead, I'm working on it... from time to time.
Yes, I just need to discover how to avoid running those tests for embroider @jherdman
I've been thinking and maybe dropping embroider tests is the best way at least for now? since it would be unexpected for embroider builds to break if they use this strategy?
I hate to do this, but I think we need to temporarily revert this strategy. I definitely merged it prematurely, and I'm not comfortable leaving the master branch in a state that's not deployable. I sincerely apologize, this is entirely my fault. Would you be amenable to working with me on a new pull request that introduces this behaviour when it's ready?
Yes I understand, again the only problem at the moment is that hbs
strategy is simply not compatible for embroider builds and I don't think it ever will as a v1 addon, as suggested by ef4 here. A whole new addon/strategy/refactor should be used instead: a v2 addon is the right way. A major version with probably breaking changes is inevitable... eventually, so there's no point in testing for embroider IMHO if we want an hbs strategy to work in the meantime.
@betocantu93 what do you think the path forward is here?
@betocantu93 what do you think the path forward is here?
Hello @MelSumner we are using this strategy on production without issues with classic builds, here's the example: https://github.com/prysmex/ember-eui/blob/master/site/ember-cli-build.js
I also added an option to include them in the addon tree, instead of public, if that's desired, so the components could be bundled, this was needed for Cordova apps, we had to bundle them since apps for android or iOS won't dynamically load extraneous JavaScript
I think the correct way to go is a new V2 addon, or even just a custom webpack svg-hbs-loader for embroider
IMHO I would leave ember-svg-jar for classic builds.
The other alternative is having a weird build pipeline which does something different if running on embroider or classic, which would be hard to maintain and superseded by the loader
@jherdman how does that sound to you?
Also for clarity, @betocantu93 are you suggesting a major version bump that is the v2 of this same addon, or are you suggesting that we create a separate addon?
@MelSumner if a reasonable path forward cannot be determined with the package as-is, I'm all for something new.
@betocantu93
- is there already an issue to track this?
- do you think that we can revamp the current addon to be a v2 addon, or should we start a different package?
@betocantu93
- is there already an issue to track this?
- do you think that we can revamp the current addon to be a v2 addon, or should we start a different package?
No, there's no issue yet.
Just my questions around the loader here: https://github.com/embroider-build/embroider/issues/746
Not sure yet, but I think the loader could be a simple webpack plugin and svg jar just focus on the UI/discovery part.
I had a PoC, but had a few issues compiling the templates, I'll try again soon.