Feather dependency

[craigrodway]

Hi! 👋

I've been thinking about #3 and ways this could be done - or achieve a similar result.

Not sure if you're aware of https://asset-packagist.org/, but essentially it allows PHP projects to pull in packages from Bower or npm registries as if they were regular PHP packages. If php-feather is updated to require the npm package for feather-icons, it should remove the need to re-build this package on every Feather update.

"require": {
	"php": ">=5.3.0",
	"npm-asset/feather-icons": ">=4"
},

This means projects that require php-feather also require npm-asset/feather-icons - which means they can use the most recently published version which this library will then use (instead of the bundled ones). This does impact the way the icons are loaded, how it is tested, and no longer requires building. But it seems to work.

This PR includes the following changes:

• Update Icons class to load the icons.json file from the npm package. This is only 52kb at the moment - so I think any performance/memory concerns of the single JSON file loaded vs loading each file on-demand separately isn't anything to be worried about. This sort of resolves Preload all icons #5 too!
• As php-feather isn't re-built, the default attributes are embedded directly into the class.

This also essentially removes the requirement on NodeJS for development/building. I haven't removed those yet, but don't mind doing that and some tidying-up if you're happy with the changes and direction. Look forward to hearing your thoughts!

[craigrodway]

Just coming back to this, have just realised this only works when the project requiring pixelrobin/php-feather also includes this in the composer.json:

"repositories": [
    {
        "type": "composer",
        "url": "https://asset-packagist.org"
    }
]

Back to the drawing board, perhaps? 🙁 🙂

[Pixelrobin]

Hey @craigrodway, thanks for the PR/suggestion! Sorry for the late response, I'm a bit inactive on GitHub at the moment. I don't have time at the moment to look into this at depth right now, but I'll try to within a few days.

[craigrodway]

Hey! No problem at all :) That's fine, no rush. I'm still not sure if it's the best way to do it, and am still looking at alternatives :)

[davidurco]

@craigrodway @Pixelrobin Hey guys can we get this running? I would really appreciate it.

[Pixelrobin]

Hello all, this project has been on the back burner for me for far too long, apologies for that, it's totally my bad.

I do appreciate your experimentation though! I think there are two outstanding concerns I have about it:

• This depends on feather not ever changing their package structure since it doesn't lock into a specific one
• I'm not 100% sure the vendor directory detection code is bulletproof enough to be used on a public package. I could be wrong though, it's something I'm open to discussion on. Maybe there was another similar project that used a pattern like this as a reference?

I like the idea of loading from a JSON file though, that seems a lot more sensible. Perhaps as a dead simple alternative to start we can version the icons in the repo as a JSON and get rid of the node to PHP build step?

[natewiebe13]

I definitely like some aspects of this. I've been looking more into what it would be like to decouple Feather Icons from the project and I think as nice as it would be there are a few reasons I'd prefer to not add the extra dependency.

• Similarly to what Pixelrobin has mentioned, this relies on Feather icons not changing their structure
• This wouldn't take advantage of php preloading introduced in 7.4 as the project doesn't include any php

What I'm currently thinking is that we use assets packagist as a dev dependency and use var_export or similar in order to build a php array that can be preloaded, also solving #5. We could then also create a recurring Github Action to run this script and open a PR as needed. I'll keep this open until we have a PR in place to supersede the work done here, but this is a fantastic move in the right direction.

Thanks for your contribution here @craigrodway!

[natewiebe13]

Superseded by #30. Thanks @craigrodway for your contribution!