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

Allow access to the Reference name and the resolved asset #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dlundgren
Copy link

While working on deduplication of assets in my own library, I found that I had to use a full copy of the AssetReference class. Since the resolve method and name properties are private, extending the class is not possible, and there are no methods available to retrieve just the asset or name of the Reference.

I'd like to add the following to the AssetReference class:

  • getName returns the name of the reference
  • getAsset returns the result of resolve

@dlundgren dlundgren requested a review from a team as a code owner December 18, 2020 05:50
@bennothommo
Copy link
Member

@dlundgren thanks for your contribution. Would you be able to provide a use case for why it would be needed to access the name and asset directly?

@dlundgren
Copy link
Author

I have something similar to WordPress' wp_enqueue_style and wp_enqueue_script that allows the assets to be registered and used as needed.

In my tests, the deeper the references are, the more likely I was to end up with duplicated content. So I was using the reference name to deduplicate the reference from deeper in the collection, without triggering the asset resolution when it's not needed.

I think I was using the asset object to replace the reference in some spots, but I no longer see that code in my repo, so I can probably ask for that later if needed.

@LukeTowers
Copy link
Member

@dlundgren would you be willing to provide a code example of how you're using these methods?

@bennothommo can you think of any reason why we should have any private properties or methods in this library at all instead of just protected ones?

@bennothommo
Copy link
Member

@LukeTowers private does have its uses - it allows a class to define methods that affect it and only it, with the presumption that if another class extends upon it, it has to provide its own implementation of the method as opposed to relying on the parent class. I find it less useful for properties (especially those that are necessary for normal class functionality, again with a presumption that class may be extended upon).

@LukeTowers
Copy link
Member

@dlundgren any updates on this?

@dlundgren
Copy link
Author

Thanks for reminding me about this, I'm still looking at using this, but I've been working on other aspects of my project at the moment.

Here is the relevant code I'm using. The Reference object has getName and getAsset as defined in this PR, and the rest of the code is basically copy pasta from Assetic\Asset\AssetReference

https://gist.github.com/dlundgren/64c9d6892d3a465f9e52fbdada317973

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.

3 participants