asset-loader icon indicating copy to clipboard operation
asset-loader copied to clipboard

Version 1.0.0 - what's blocking this?

Open Sephsekla opened this issue 2 years ago • 4 comments

At time of writing, the current release is v0.6.0.

This is causing a few headaches with updates, as Composer (understandably) treats anything before 1.0 as pre-release.

i.e. while a constraint of ^1.0.0 will happily update to 1.1.0, ^0.5.0 will not update to 0.6.0.

Given that this library is already in use by a lot of production projects and is pretty stable, I'd argue it's no longer something I'd call pre-release. What would we still like to achieve before declaring it stable, and updating to v1.0.0?

  • Documentation?
  • Additional filters and actions?

Sephsekla avatar Jun 16 '22 10:06 Sephsekla

Great question. There aren't many TODOs on my end, but for a long while now I've wanted to make a proper docs site for this à la what we have for https://humanmade.github.io/webpack-helpers — that shouldn't be a blocker for 1.0, but would go along with that milestone nicely.

I'm curious what if any functionality, filters, or API changes people feel might be needed, if we're going to pin a full version number onto this repo.

kadamwhite avatar Jun 16 '22 10:06 kadamwhite

Probably the biggest risk / gotcha I see with Asset Loader right now is that if you try to enqueue a file and there is no manifest on disk, get_asset_manifest() returns null which is not a string which causes a 500.

Argument 1 passed to Asset_Loader\enqueue_asset() must be of the type string, null given

This is probably a rough edge we should smooth off. I'll spin up an issue for it. Edit: Done, #41.

kadamwhite avatar Jun 16 '22 11:06 kadamwhite

One other question to the group here: do y'all have opinions about whether all of our main API methods should be exposed directly on Asset_Loader? for example get_active_manifest is within Asset_Loader\Manifest so you need to use two things to use it alongside enqueue_asset. Would it be better to expose that at the top level, so that we can consider all of our Asset_Loader\* functions to be the "public API"?

kadamwhite avatar Jun 16 '22 11:06 kadamwhite

do y'all have opinions about whether all of our main API methods should be exposed directly on Asset_Loader?

I think that makes sense. The only function I find myself using that's not in the Asset_Loader namespace is that Manifest\get_active_manifest() example you gave, and it feels to me like that should be wrapped in a top-level function.

It would feel more natural to me if Asset_Loader\register_asset and Asset_Loader\enqueue_asset could accept either a string or an array for the manifest path argument; and if the manifest path is passed as an array, the functions automatically resolve them using Manifest\get_active_manifest().

goldenapples avatar Jun 16 '22 13:06 goldenapples