Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolveAsset interface via new svg-jar service #189

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

betocantu93
Copy link
Contributor

@betocantu93 betocantu93 commented Mar 24, 2021

In order to avoid locked in to a particular addon for resolving assets, let consumers choose how they resolve assets i.e. ember-cli-resolve-asset or ember-cli-ifa for the hbs strategy the host app must implement /services/svg-jar's resolveAsset method, this PR also guards FastBoot usage, honestly because I don't really understand how dynamic imports work in node and ember-cli-fastboot

Example using ember-cli-ifa

import EmberSvgJarService from 'ember-svg-jar/services/svg-jar';

export default class SvgJarService extends EmberSvgJarService {
  @service assetMap; //comes from ember-cli-ifa

 //interface 
  resolveAsset(path) {
    return this.assetMap.resolve(path);
  }
}

Example using ember-cli-resolve-asset

import EmberSvgJarService from 'ember-svg-jar/services/svg-jar';
import resolveAsset from 'ember-cli-resolve-asset';

export default class SvgJarService extends EmberSvgJarService {
 //interface 
  resolveAsset(path) {
    return resolveAsset(path);
  }
}

Example without any addon, this could potentially be the default one and avoid having to declare the service altogether

import EmberSvgJarService from 'ember-svg-jar/services/svg-jar';
import resolveAsset from 'ember-cli-resolve-asset';

export default class SvgJarService extends EmberSvgJarService {
 //interface 
  resolveAsset(path) {
     return path;
  }
}

@betocantu93
Copy link
Contributor Author

maybe the guard vs fastboot broke embroider? 🤔

@betocantu93
Copy link
Contributor Author

The ultimate intention of this PR is to give more flexibility and enable proper documentation of usage

@betocantu93
Copy link
Contributor Author

Removed any dependency to resolves addons with this interface approach, I think this might be the best approach, and document how you could integrate any of these solutions...

@betocantu93 betocantu93 changed the title Resolve asset interface resolveAsset interface via new svg-jar service Mar 25, 2021
@betocantu93
Copy link
Contributor Author

betocantu93 commented Mar 27, 2021

I updated the ember-try for embroider scenarios, but I honestly don't think this PR should be stopped by the current state of embroider, what I mean is that hbs strategy does not currently work for embroider at all for a couple of reasons that I'd try to figure out from time to time, still here's the issue I opened at embroider: embroider-build/embroider#746

One way to get the tests green is to just literally remove the usage of hbs strategy for tests and/or somehow scope them behind a non-embroider scenarios

@betocantu93 betocantu93 mentioned this pull request Mar 28, 2021
@jherdman
Copy link
Collaborator

This is going to take a little while to review.

@betocantu93
Copy link
Contributor Author

betocantu93 commented Mar 29, 2021

@jherdman I can split this PR, leave all of the embroider part out of it, the important thing is the service and the interface, and we would need to state that this strategy currently doesn't work for embroider.

@jherdman
Copy link
Collaborator

Let's split this PR in two as you suggested. I'm not familiar with Embroider so it'll take a while to get up to speed on that aspect of this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants