Open LeXofLeviafan opened 3 years ago
@Gozala what do you think about giving @LeXofLeviafan commit access to this repository? These contributions are outside my ability to make a judgement call on.
@Gozala what do you think about giving @LeXofLeviafan commit access to this repository? These contributions are outside my ability to make a judgement call on.
I don't mind giving @LeXofLeviafan commit access. That said, I do think someone should make judgement calls or this will serve no one at all. I would personally be much more comfortable if you were to make those calls given that you've taken a lead as opposed to someone new like @LeXofLeviafan (no offense)
Turns out I am unable to grant anyone admin rights on repos under my github name, so I would like to propose to take this as an opportunity and transfer this repo to a new github org where I can grant @chr15m full admin rights and subsequently power to give write access to whoever it would make sense.
As of proposed changes, I do not believe I am in a position to make any calls anymore as I have not committed to this project in years. All I can say is that it not aligned with an original vision, I intentionally was trying to fall on the side of simplicity over flexibility and that was a reason why only subset of module import syntax from Clojure was ever implemented. But then again, I do not vision at a time matters all that much today.
@LeXofLeviafan I'm actually curios what use cases does this new import syntax address that were not possible before ?
@Gozala I agree that it's preferable to have centralised decision-making on which commits make it into master, as well as that it's better to leave it in to someone more familiar with the project.
As for your question… Before I decided to minimise runtime dependencies of my mreframe project I tried to write it in Wisp, and when I started to write tests I spent an awful lot of effort trying to import the main sources correctly (as well as making subpackages work together); and even though I ended up restructuring the project, it still took some weird workarounds to make it work (what actually did work) due to a surprising amount of quirks in the import system.
As for your question… Before I decided to minimise runtime dependencies of my mreframe project I tried to write it in Wisp, and when I started to write tests I spent an awful lot of effort trying to import the main sources correctly (as well as making subpackages work together); and even though I ended up restructuring the project, it still took some weird workarounds to make it work (what actually did work) due to a surprising amount of quirks in the import system.
I do not have enough context here to provide meaningful feedback, but looking at your test/atom.coffee module I would have expected following to Just Work™️:
(ns mreframe.test.atom
"tests form atom"
(:require
[ospec :as o]
[mreframe.src.atom :refer [deref resetVals reset swap swapVals compareAndSet atom]]
[mreframe.src.reagent :as r]))
If it did not that sounds like a bug. If it was not clear that above was a way to go than probably docs on importing could be improved to clarify.
Like I said, the current (flat) package structure with nested module implementations came after quite a few design changes; originally I was trying to get closer to the package names one would import in Clojure (for sake of “more compatible” code). That caused a lot of problems with relative imports detection in Wisp… which could all be easily avoided if I had the option to specify those paths directly.
The worst issue was that src/
was the root in the original sources structure, which meant trying to import any of these modules from outside of it… technically worked, but only if the module didn't try to import anything from outside of it.
Like I said, the current (flat) package structure with nested module implementations came after quite a few design changes; originally I was trying to get closer to the package names one would import in Clojure (for sake of “more compatible” code). That caused a lot of problems with relative imports detection in Wisp… which could all be easily avoided if I had the option to specify those paths directly.
I'm sorry it did not worked. I understand those could have been addressed with relative imports but then again it was explicit choice not to support that. That is not to say that decision needs to be maintained going forward, just trying to clarify that it was not an accident but by a design.
The worst issue was that
src/
was the root in the original sources structure, which meant trying to import any of these modules from outside of it… technically worked, but only if the module didn't try to import anything from outside of it.
You could also choose to not make src
the root. Modules in src
could have had namespacse like mreframe.src.atom
which would have enabled referring anything under mreframe
root. Namespace encoding of paths was intended to be a sugar for actual paths mreframe.src.atom -> mrefram/src/atom
and partially motivated by the fact that I never liked that node did not allowed referring to the package by it's name and forced relative paths instead.
I am sorry @LeXofLeviafan wisp did not lived up to your expectations. And I am grateful that you even took time to try to fix it so it would meet your needs. That said, I am still not convinced that importing via relative path or ability to diverge namespaces from paths is an improvement. However I'm not the authority nor I have maintained this repository for years so my opinions are irrelevant, at best context that lead to specific decisions might help guide new ones but even that is probably a stretch.
I'm not the authority
As the original designer of Wisp I think your input is super valuable. Currently there is no authority on this codebase (certainly not myself) and so it will have to be "rough consensus and running code" as they say. For this particular PR the running code is nicely satisfied but I don't think "rough consensus" is quite there yet.
You could also choose to not make
src
the root.
That makes it an outright limitation of the language, which is not something that even should exist in it; placing source root in a subdirectory is a legitimate option, and making it unavailable just because something doesn't work in the language compiler (as opposed to fixing the problem itself) sounds like a bad idea to me.
I've been fiddling with a project and came to conclusion that in some cases, allowing for custom require paths in Wisp would be quite helpful (or rather, not having such a fallback option can get seriously frustrating at times).
The idea is based on shadow-cljs implementation of JS imports.