-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace jQuery with Halogen #199
Comments
This relates to the work @milesfrain has been doing to add new features to Try PureScript. He migrated the application to use Halogen as part of that work, but as there were several other changes made at the same time it's a bit difficult to review properly. By making a new PR which only migrates the application to React Basic or Halogen we could at least have a more solid base to evaluate new features one by one. |
I'd be up for a rewrite in either Halogen or React. It would also be nice to resolve #197 before tackling this issue. |
I am personally in favor of migrating away from ad-hoc jQuery bindings, but I think we should clearly outline what would make it a successful transition. jQuery is advantageous, because the SPA needs of try-purs are very conservative, and I think that's a good thing. I consider it an anti-goal to iterate on try-purs to the point of being an IDE in the browser, and that we should focus on it being a small-demo showcase and way to share quick snippets. Not that something like that should not exist somewhere, just that I don't think that should be the goal of the core team infrastructure. As an example, Ellie exists for elm, and is able to iterate independently and grow based upon the creator's vision for editing and sharing Elm code, which is separate from the language infrastructure's development. The official demo case for TypeScript is very conservative, and is basically only console logging. Additionally, I also consider it in an anti-goal for try-purs to be a beacon example for modern SPA development in PureScript. That's not to say that Halogen or React are bad choices or the wrong choices, just that I do not consider that to be a contributing factor. My personal criteria are:
I consider "controversial architecture decisions" to largely mean fancy types like Run, Variant, Cofree, etc. That is, the goal is not to showcase all the fancy things you can do in PureScript, it should be very straightforward for someone to read, understand, and review. I want to be clear that I don't necessarily consider Halogen or React to be immediately rejected on those terms, just that I think we should prioritize those points. |
Big 👍 to everything @natefaubion has said, especially the anti-goals. I am open to moving away from jQuery to either Halogen or react-basic too, provided that the PR to switch over doesn't do anything else at the same time. A point against react-basic is that in order to be consistent with the "no bundling step" criterion we'd have to do a little bit of trickery with a require shim or something, because react-basic uses I am not entirely convinced that the ContT -> Aff switch should happen at the same time; to me it seems preferable to do the ContT -> Aff switch on its own first, and to do the framework switch afterwards. |
I think these points are all reasonable and all possible while transitioning away from jQuery.
I agree with this scope for try-purs. I think that switching to another framework and introducing the features that @milesfrain has proposed are each within this scope, too; the biggest ambiguity is probably to do with what features will enable a 'small demo showcase' as folks may have different ideas about what is sufficient to pull that off. For example, I think it's a good thing that small demos can present a UI instead of only displaying logs (as TypeScript does), because I think it's good to be able to share small issues or demos of things built in Halogen or React Basic or whatever library (one use case being the PureScript Cookbook).
I agree. I think the explicit goal for try-purs should be that it is simple and maintainable. I think jQuery might be a little too far in the opposite direction on the "beacon of modern SPA development" spectrum :).
This sounds like a vote for Halogen (or something else built on the Halogen VDom, like Spork), as that can be done with only stock tooling.
Definitely a good idea. This isn't a complicated application. I agree that this doesn't rule out a non-jQuery solution and either stock Halogen or any other PS framework can satisfy this by just using the library out of the box.
I'm on board with smaller bundles, though that complicates the decision somewhat; a Halogen and a React application can have comparable bundle sizes, but nothing's going to beat say just using DOM bindings directly. How important is this, and do you have any sense of what threshold you might not want to cross? For context, the entire Real World Halogen application is about 115kb gzipped. Are there other libraries you think are worth considering with this in mind?
OK. If we go this way, then we could have a series of PRs essentially doing:
Does that seem like an acceptable plan? |
Again, I want to reiterate that I'm not trying to exclude anything in particular and I don't have a specific threshold, just that it should be a priority when considering things. I couldn't tell you what the comparison is between React and Halogen in this case. Both are non-trivial. I think it is likely that Spork would yield the smallest bundle between these three by a significant margin. I don't plan on it going anywhere, but saying it has little community mindshare would be an understatement. |
I don't know about this specific case, but they're in the same ballpark in general. Using Real World as an example again I believe Halogen is within 10kb or so of React. But I agree they're non-trivial.
This is the main reason I hesitate a little wrt Spork. It's simpler, would help ensure no fancy shenanigans in the internals, it'll have smaller bundles, can be done with stock tooling alone, but it's not as widely known or used as something like Halogen. That said, I think it's an acceptable choice. |
This is just me, and venturing into off-topic so I don't think we should discuss it more in this thread, but I think the basic workflow for try-purs should stay as-is:
I think the features @milesfrain has been working on are within a reasonable scope. I do not consider it in scope to include multiple modules, multiple language editors (HTML, CSS, and especially foreign modules), or anything outside of editing PureScript code. If we want to offer additional features, we should add them to the |
I've reopened #179 in light of this discussion. |
With all of this in mind, it sounds like we're (at least lukewarmly) in favor of choosing something like Halogen or Spork to transition away from jQuery and before moving on to add features. If we are able to settle on a particular library then I'm happy to update this issue to reflect that choice and label it for someone to work on. @hdgarrood @natefaubion Do either of y'all have a specific preference? Are there outstanding concerns that I've missed that we should address before deciding? |
I think that Halogen would be the easiest to bundle and deploy (among React and Halogen). Unfortunately, I would put Halogen Hooks into the "controversial" category, just because I consider it still experimental, it is a third-party library, it adds additional bundle overhead, and I do not consider the needs of try-purs to require much more than a single component. |
I agree that we shouldn't use Halogen Hooks in Try PureScript. While Halogen isn't within the PureScript organization it is at least a well-vetted, popular library that's been maintained for years |
Given the thumbs up, and as I am on board with Halogen as well, I'm going to make this assignable and update the description. |
Yes, that sounds great to me.
It seems like we are settling on Halogen without Hooks, which I think I'd be happy with too. Just to be completely clear, though, this isn't a hard commitment - while I do think PRs doing these things in this order are likely to be merged, there are no guarantees. |
Try PureScript currently relies on the
jquery
library and some extras defined inJQuery.Extras
in this application. It uses these bindings largely in the client/Main.purs file which manages the Try PureScript window and iframe.There are a few drawbacks to using jQuery:
There are some advantages of jQuery, too:
I think that using jQuery is a net negative for new folks trying to understand Try PureScript and contribute new features to it. Ultimately it’ll be PureScript developers contributing and my sense is that within the community frameworks like React and Halogen are considerably more popular — and while jQuery may be much more widely used and understood than those in general, within the community there are a lot of folks who weren’t web developers before PureScript (like myself) and are not as familiar with things like jQuery.
I also think that the use of jQuery lends to a style of programming that makes this app harder to grow and maintain over time — specifically the imperative style and having to know what a selector means rather than the more PureScript-y declarative nature of React or Halogen.
For these reasons I think we should explore rewriting Try PureScript in either React Basic or Halogen. This rewrite would add no new features and faithfully preserve how Try PureScript currently works, except in the React style or the Halogen style.
(Also — I think it makes sense to switch
ContT
forAff
at the same time, for ease of development. We’d already agreed that this is worth doing in #179 and it will probably be easier to make both changes together rather than change one at a time.)I don’t personally prefer React Basic or Halogen over one another, but I think those two are the most widely used and understood of the SPA libraries. I’m a little partial to Halogen as feels nice to use a fully-PureScript framework to write a PureScript core resource like Try PureScript (and then we can use a PureScript-only toolchain), but I understand that React is the more widely understood library.
The text was updated successfully, but these errors were encountered: