Add selectMountPoints to API #3
Conversation
There was a problem hiding this comment.
I'm down with this.
Is it worth exporting INSTANCE_ID, or better to trust that 'owned' mount points are entirely handled internally?
Also, the pedant in me would change selectMountPoints to selectMounts unless we wanted to apply the "points" terminology to the whole API surface area. I'm not against that (it's probably better), but it'd be a breaking change.
The sooner we address that, and your other API change propositions, the better. Usage is pretty small right now, so it's the best time to make breaking changes.
|
My instinct on the export of I'd also like make it so the default is to ensure block mounts. That should be fine right? You'd just have to add options for the usage in legacy interactives mentioned. Happy to go with the original nomenclature, I'll update the function name. What are your thoughts on removing/privatising other functions? I feel like you're not that into it. |
|
One more thing that doesn't pertain specifically to this pull request, but that I wanted to write down somewhere: something in the build process here isn't handling transpilation of the spread operator correctly. I originally had this line as It was a hard one to track down and I still don't know how to write a test for it, because it was passing all the tests. Assume there's something up with the babel config, but went with the simpler solution. |
|
Gonna merge this soon if there are no other comments. Still keen to hear what you think about removing some of the other public methods @colingourlay. |
|
Still happy with the latest changes. I'm all for trimming the API down, because it'll discourage a wide range of uses that'd be hard to re-focus later. Getting access to selectors was only really useful when the library itself wasn't doing any DOM selection, but now that it does, those kinds of functions can go. |
Create a
selectMlountPointsfunction which wraps up a bunch of other functionality, including:This is a non-breaking change, but ultimately I think we should encurage use of this function as
the main API method and deprecate or remove some of the other methods that don't handle marking
mount points as used.
There may be better ways to implement this, but it's a start for disucssion.
This also gets pretty close to 100% test coverage.