Remove Dependency of PathKit for External Programs
HelloFillip opened this issue · 17 comments
There's no logical reason that a consuming program would need PathKit. An overloaded init could be included to support a plain String representation of a path that is internally converted to a PathKit Path.
This would remove an unnecessary third-party dependency.
public init(from paths: [String]) {
...
}
An overloaded init could be included to support a plain String representation of a path that is internally converted to a PathKit Path.
-
Path is string literal convertible. You can pass literal strings to the same method. As show in the documentation (https://stencil.fuller.li/en/latest/api.html#filesystemloader)
FileSystemLoader(paths: ["./templates"])
-
Path adds an additional type hint so it is clear what the intended value is.
This would remove an unnecessary third-party dependency
PathKit is not a third-party dependency. It is first party, I made both Stencil and PathKit.
I agree in removing PathKit dependency. The location of the template has nothing to do with the actual template parsing engine which is the core purpose. First-party, third-party, etc it's still dependency and removing would make Stencil much more portable and lighter.
- PathKit is a very small dependency, so "much lighter" isn't an argument.
- PathKit works on Linux, so it is portable.
- Stencil needs to load files, so it needs filesystem access, in a platform independent way. Foundation is a bit lacking in this sense, especially with path transformations, etc...
How about merging it in then if it's so important to the core functionality. Juggling dependencies and waiting for both to be in sync as far as versioning and compatibility is a bit clumsy.
How are we juggling dependencies? Stencil defines which PathKit version it needs, and is compatible with. Your package manager (CocoaPods/SPM) will install the correct version accordingly.
PathKit is used by other projects, it makes no sense to directly merge it into Stencil, as that would mean we'd need to maintain 2 codebases.
I see your points. It's just that having a library depending on another library creates more moving parts. A self-contained library would be easier to manage and consume. Even if just extracting the piece of PathKit that Stencil needs would be great.
Like I mentioned before, you don't need to manage, nor consume the library yourself. You tell CocoaPods or SPM you need Stencil 0.13, and they get it for you + any dependencies.
Maybe I'm bias because I use Carthage, or others who who just embed it. Just another project or framework to juggle.
It's clearly a no-go to merge PathKit into Stencil. PathKit is used independently by many other libraries, that don't use Stencil, only PathKit.
As @djbe said, there's nothing to juggle. Any good dependency manager does that for you. e.g. you put pod 'Stencil' in your Podfile (without having to add pod 'PathKit' at all) and CocoaPods will resolve transitive dependencies and automatically download, install and import PathKit for you. Same with SPM, you put Stencil as a dependency in your Package.swift and SPM will resolve PathKit as a transitive dependencies and integrate it automatically.
By the way, the resolution of the dependency graph and the transitive dependencies is a key feature of any good Dependency Manager, so this is its job to do so, not the developer's, also because resolving a graph with many different libraries sharing transitive dependencies in different versions can become complex and lead to duplicate symbols and incompatible versions if done wrong.
Carthage is the exception in the world of dependency managers tbh, as it's the only DM I know of (even comparing with DMs for other languages like ruby, nodeJS or PHP) that doesn't resolve transitive dependencies for you, and force you to explicitly list transitive dependencies yourself. Maybe if that part of Carthage doesn't work for you you might be better off trying solutions like CocoaPods-Rome, which is a good solution in-between the two worlds (using a Podfile and CocoaPods, so having the resolution of transitive dependencies done by CP, but with a plugin that makes CocoaPods works similarly to Carthage, only pre-compiling the .framework for you to avoid rebuilding them every time, and letting you integrate them yourself, like you do with Carthage)
Anyway, Stencil and PathKit are two different projects that evolve independently anyway, PathKit being sometimes updated on its end with nothing related to Stencil, and used by frameworks other than Stencil, so they clearly live separate lives, and integrating PathKit into Stencil, and inter-locking the version of PathKit with Stencil's would lead to duplicate symbols or incompatibilities with many projects using other libs depending on PathKit but with different version constraints (that the whole semantic versioning dance allows to avoid, thanks to loose version coupling)
I think FileSystemLoader is the issue we don't see eye-to-eye with. My templates are loaded from a database so I don't need to have file disk support like a web server would. Bundling a custom loader with Stencil that many wouldn't use then forcing a dependency on it seems presumptuous. If I took out FileSystemLoader from the source code, everything compiles without PathKit. Having an outside dependency like Stencil+FileLoader, kind of like a pluggable marketplace of some sorts, like Alamofire for example, seems to strip down Stencil down to barebones and focus on the core purpose of a template parser engine.
Carthage is the exception in the world of dependency managers tbh, as it's the only DM I know of (even comparing with DMs for other languages like ruby, nodeJS or PHP) that doesn't resolve transitive dependencies for you, and force you to explicitly list transitive dependencies yourself.
I'm sorry it's a bit off-topic, but the statement above is completely wrong. Carthage successfully handle transitive dependencies. So a Cartfile with the single line of github "ReactiveCocoa/ReactiveCocoa" is resolved to a Cartfile.resolved with the following contents automatically:
github "ReactiveCocoa/ReactiveCocoa" "8.0.0"
github "ReactiveCocoa/ReactiveSwift" "4.0.0"
github "antitypical/Result" "4.0.0"
No need to list the all three dependencies to a Cartfile by your hand.
See that's something else entirely. You're welcome to create a PR to add a Cartfile to the project, if that solves it for you, but too many projects already depend on Stencil as it is, with the file mgmt stuff.
@ikesyo Ah that's good news, sorry for the wrong statement then, I stand corrected
Then I'm lost, as I assumed that from what @basememara described above… so @basememara @siilime what's the core problem then, if Carthage adds the transitive dependencies if PathKit for you, why do you need so much to remove it as a dependency… while it's needed for Stencil to work anyway?
I also don't see any problem with Stencil depending on PathKit unless it is blocking development of Stencil or something.
AFAIK only Cocoapods supports sub specs that we could use for breaking this up, but as it is not the only package manager we support, we shouldn't probably do that.
I imagine most of the users of Stencil to load templates from the file system so IMO it is a better experience for the major audience instead of splitting it up and making most of the users to add another dependency, which in fact will result in two dependencies, so 3 in total (Stencil + Stencil + FileLoader + PathKit) vs 2 dependencies (Stencil + PathKit).
..actually (Stencil + Stencil + FileLoader + PathKit) vs (Stencil).
Really didn't mean to make this about Carthage. Was just advocating Stencil not to have any external dependencies.
@basememara I agree in general, it's definitely a good thing for a library but we also should be pragmatic about that. For example Sourcery, and I suppose SwiftGen, tools which very heavily rely on Stencil, also use PathKit as their direct dependency, so it makes sense to have it as a separate library.
Of course, don't get me wrong, PathKit is awesome and should be its own library. I think what I'm trying to say is if we could create a FileSystemLoader without any external help then it would make Stencil self contained.