Skip to content
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

Separate Generated and Non-Generated Source Files in SBT Build #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

YakimaProgrammer
Copy link
Contributor

@YakimaProgrammer YakimaProgrammer commented Jun 24, 2024

Hello!
This is our (myself and @markschaake) first pull request and we were getting our feet wet with this project. We were taking a look at the build and had a couple of ideas for teasing apart generated and nongenerated sources. We'd love if you took a look and shared your thoughts!
These changes have a couple of tradeoffs:

  • SBT caches and generates source files as needed instead of intermingling them with nongenerated sources
  • Generated code is no longer part of version control
  • Hopefully, there will be a little less mental overhead by making generated sources SBT's area of concern (no longer need to read comment headers in files to differentiate, for example)

- Modified `build.sbt` to use `Compile / sourceManaged` for the output directory of autogenerated sources.
- Modified `ShoelaceGenerator.scala` to return a `List[File]` of generated sources.
@raquo
Copy link
Owner

raquo commented Jun 24, 2024

Heyo, sorry I didn't have time to respond to Mark's related comments before you started working on this.

The generated files in all of my projects are committed to git, and that's very much by design. The main reasons for this are:

  • End users don't care if the files are generated. To them it's just source code, and when the code they're using is missing from git, it gets confusing. For example, as a user I often want to find definitions of classes and methods that I'm using, and sometimes the easiest way to do that is to use github code search. I also sometimes just want to browse all available code in a project to see what it does and how it does it. If the generated source exists only in maven artifacts, it can be hard to access.
  • As a maintainer, I want to know how the changes in the generator affect the generated code. Having the generated source in git gives me immediate diffs in the IDE, and when reviewing PRs. This gives me confidence in the code I'm releasing without requiring 100% test coverage and manual lookups inside the target dir.
  • For projects that are published for Scala.js, we need to publish all source code to github, because Scala.js source maps (the technology that lets you see original Scala source code in browser debugging tools instead of the generated JS code) needs a way to access the Scala source files, and giving it a URL to the source file on github is the easiest way to accomplish that.

Can we keep the generated files where they are, but still add generateShoelace to Compile / sourceGenerators? It would be a non-canonical usage of that sbt key, and TBH I'm not sure if we will actually get anything that way (e.g. clean won't clean the generated files, not sure about the rest).

If you want to separate the generated code from non-generated code in the source, please note that I want to keep the generated code where it is, because it's all user-facing, and I want to let users easily import everything they need with just a single com.raquo.laminar.shoelace.sl, and then use it as sl.Button(...). If you want, you can still separate the non-generated files like Shoelace.scala and WebComponent.scala, moving them into e.g. sl/common subdirectory, and create type and val aliases to them in package.scala so that they remain accessible via e.g. sl.Shoelace.setBasePath(...). We do things that way in Laminar for example, because Laminar is more complex and can't have all the user-facing types defined in the same top level package. For this smaller project, personally I could take it or leave it, either way is fine.

@markschaake
Copy link

markschaake commented Jun 25, 2024

@raquo thanks, I think we understand the requirements and reasons. We were looking at the build first as a way to get familiar with project and hopefully fix a broken window or two along the way. Thank you for your patience and help while we get familiar. I believe @YakimaProgrammer is already started working on your "#nc" comments in ShoelaceGenerator - which I think is where the functional gains are to be had (yay!).

Regarding this PR...

I do think there might be a way to solve "all the issues", where those issues are:

  • caching problems with generated code not managed by SBT (Magnus hit an issue with this he said)
  • generated code ending up in the same directory as hand-managed code
  • not removing generated code from source control (understood why this is important)
  • generated code not removed on clean
  • not needing to hand craft an Aliases file

I think this can be achieved with something simple like:

// build.sbt

val generatedDir = settingKey[File]("Directory in which to generate sources")

// settings
...
.settings(
  generatedDir := (Compile / scalaSource).value / "com" / "raquo" / "laminar" / "shoelace" / "sl" / "generated",
  generateShoelace := new ShoelaceGenerator(
    onlineSourceRoot = "https://github.com/raquo/laminar-shoelace-components/blob/master",
    customElementsJsonPath = "node_modules/@shoelace-style/shoelace/dist/custom-elements.json",
    baseOutputDirectoryPath = "src/main/scala/com/raquo/laminar/shoelace/sl",
    baseOutputDirectoryPath = generatedDir.value, // <- this is now in source control
    baseOutputPackagePath = "com.raquo.laminar.shoelace.sl" // <- this stays the same
  ).generate(),
  // Now to make sure we trash generated files on clean:
  (Compile / clean) := {
    (Compile / clean).value // make sure run regular clean (like super.clean)
    val dir = generatedDir.value
    IO.delete(dir.listFiles().toList)
  }

I think this would work and not require adding an Aliases file, since SBT doesn't enforce package directory structure so generated code in "generated" can still be in the same scala package as those sources in its parent directory. But maybe I'm missing something?

@raquo
Copy link
Owner

raquo commented Jun 26, 2024

@markschaake Indeed those would be good bullet points to fix! I don't think we can do the generated directory though. Even if sbt does not enforce it, Scala users generally expect the package structure to match directory structure. I would rather not break that expectation, even if it's obvious to experienced users that the generated files are hiding under generated.

In Airstream, I do have some code in a generated directory, but in that case we can afford to have an ugly generated segment in the package name, because those implicit classes are not really user-facing – as a user, you would use their methods, but you never import those classes by name, so you would never be exposed to the ugliness of having generated in the package name.

In this project, we can't do this, because we want a simple and pretty import path, and easy references to e.g. sl.Button.

These constraints don't leave us with a lot of options. One of them is the Aliases file, and another option is keeping all files in the same directory, but marking the generated files somehow, and then adding logic to the clean command that finds all marked files and deletes them. Could be something as simple as a #deleteOnSbtClean comment in each generated file (and some sbt code that deletes all .scala files containing it on clean), or alternatively maybe we could make sbt keep track of which files it generated, and delete files from that list during clean. But that would mean managing state externally to the files. Seems complicated and bug-prone, at first glance.

Personally I think Aliases has the best cost / benefit ratio. Its main downside is that when the end user sees e.g. sl.Shoelace in their IDE and clicks "go to definition", the IDE sends them to val Shoelace in the Aliases file, where they need to click through once more to get to the actual underlying definition. But, sl.Shoelace and other non-generated types in this project would be used very rarely in user code (e.g. maybe they'll call sl.Shoelace.setDefaultIconLibrary once in their app – it's not something they'll need to do repeatedly in every component), so the tradeoff seems worth it to me. But if you want to go for one of the alternative options, that should be fine too I think, if it's not too complicated.


Personally, when I'm working on this shoelace project, my biggest gripe is that I need to remember to run reload before running generateShoelace when making changes to the generator, since it's all under /project. I solve this by running reload; generateShoelace and then using the shortcut to run the previous command. But it's definitely annoying and error-prone. I don't know if calling reload can be automated or not, or if it's a even good idea to do that. If you're running into that annoyance too, feel free to try and do something about it as well. I just don't know sbt well enough to know good from bad with such things.

@YakimaProgrammer
Copy link
Contributor Author

Thank you for sharing your perspective. I understand the structure and vision for this project a lot better now. I'm looking forward to the road ahead as we work on this project!

@PerWiklander
Copy link

Sorry for maybe raising the dead.

My understanding is that we have:

Two kinds of projects:

  1. A tool that creates a ui-lib from a component spec + some extra Scala code
  2. A resulting Laminar Shoelace UI lib (ui-lib), that an end user can depend on to create an application.

There are three types of code here:

  1. Parser / Generator: This reads the spec and generates Scala code.
  2. Generated code: The bulk of the ui-lib. This can always be recreated using a specific version of (1)
  3. Helpers / extras: Non generated Scala code that is needed at run time in the ui-lib. Probably things that the generated code calls or extends in various ways.

There are two types of users

  1. Tooling / ui-lib developers:
    Here there is a want to be sure what is hand crafted and what is generated. Generated code should always be regenerated after changes have been made to the tooling and it should not be manually changed by mistake etc.

  2. ui-lib users:
    Here we want the complete source code for the ui-lib (but not the generator) in an available, browsable way with complete version history that at least matches the released versions of the ui-lib, but preferably with enough granularity to be able to see why changes were made. These users do not care at all about who or what wrote the code.

Proposed solution

Separate the parts into different sbt modules.

  • project:
    The tooling code, like today.
  • common/src/main/scala/com...:
    The helper / glue code
  • common/src/test/scala/com...:
    Tests for the glue code. Examples of how generated code would use the glue code.
  • ui-lib/src/main/scala/com...:
    The source of the ui-lib

The generator would output code into the ui-lib module.
The glue code can either be used by the ui-lib depending on that module, or by copying the code into the ui-lib module, as part of the build.

When all code is in the ui-lib module, it can be built and published to maven and so on.
ui-lib users can browse all the source in the same place etc.
sbt clean can wipe the whole ui-lib module if needed.

Other than this I would keep the glue code in a separate package, like

  • com.raquo.laminar.shoelace.common: glue code
  • com.raquo.laminar.shoelace.sl: the generated components

this is not strictly needed, but makes the life of tooling devs easier.

@raquo
Copy link
Owner

raquo commented Oct 17, 2024

@PerWiklander That seems like a good assessment of use cases.

I don't really understand the value of creating multiple projects for common and ui-lib. I think it will be easier for end users if all the code is under sl – the generated files directly under sl, and the common files under sl/common. (We can even skip my original Aliases suggestion, if that part is controversial.) Then the users can just import sl once, and have all they need. And they will see all the source code under sl, all together in one project.

I may be forgetting some nuance that we've previously talked about, but I think this should satisfy the tooling developers as well. We would just need to wire an sbt task that would delete all sl/*.scala files except package.scala (which is empty but is required for IntelliJ autocomplete ergonomics).

If you meant this as a way to support multiple ui-libs – then yes, we'd need something like that. In that case, I think each ui-lib should exist in a separate repo – separate both from each other and from the tooling, and the common files should be copied over either manually or via the generator script into each ui-lib repo. Basically, similar to how Scala DOM Types today acts as the shared generator for Laminar and other UI libraries. That way, end users don't need to consult any more upstream projects – all the end-user code for laminar-shoelace will be in that repo, nothing more, nothing less, and similar to tyrian-shoelace et al. It's also easier to develop Scala DOM Types that way because in its own repo, the code is just regular Scala code, it's not under project.

This way, commits to these separate ui-lib repos with updated generated code would generally attribute the changes not to a specific commit in the generator project, but to a specific release of it, similarly to how we attribute changes in Laminar's DOM definitions to a specific release of Scala DOM Types. I feel like this generally provides enough granularity, as the updates in generated code are generally trivial, and the generator releases are small in scope, usually just adding support for more props.

However, the multi-lib setup is more of a long term plan. We haven't yet decoupled the Shoelace generator from Laminar, it needs to be made more configurable to support outputting code for other UI libraries.

@PerWiklander
Copy link

This:

"If you meant this as a way to support multiple ui-libs – then yes, we'd need something like that. In that case, I think each ui-lib should exist in a separate repo".

I just didn't want to suggest that at first, since it's more work to get it going.

I also see a need to generate e.g. Scala tags versions for server side rendering. It would be nice to have the same API for the components regardless of the intended output. Or is there a server side story for Laminar these days?

@raquo
Copy link
Owner

raquo commented Oct 18, 2024

I plan to eventually implement ScalaTags-like rendering functionality for Laminar on the backend. It won't directly support reactive stuff (anything with Airstream or anything with <-- / -->), but it will let you write static HTML templates much like ScalaTags does, with a few extra integration helpers for hydrating such templates from the client. See raquo/Laminar#128 No promises as to when this will happen though.

@PerWiklander
Copy link

I plan to eventually implement ScalaTags-like rendering functionality for Laminar on the backend. It won't directly support reactive stuff (anything with Airstream or anything with <-- / -->),

Would it be possible to have the same API though, but just make any reactive stuff a noop?
I don't know if its a good idea, but I was thinking of the possibility to create components once and cross compile them to js and jvm.

@raquo
Copy link
Owner

raquo commented Oct 18, 2024

I don't know, I went down that rabbit hole a while ago, and my general conclusion was that it may be possible, but it's not trivial, and I don't know if the outcome would actually be desirable. If we're going down that path, we could even implement the parts of the DOM that Laminar needs internally (basically implement a small subset of the jsdom project on the JVM), and make Airstream run on the JVM (in a single thread at least). But I'm not even sure that it would be useful in principle, let alone useful enough to warrant the effort.

I think that trying to fake a JS/browser environment on the JVM, and inevitably leaving exposed rough edges one way or another, is not as rewarding of a destination as it may appear. The issue I linked offers a much simpler alternative that is admittedly less ergonomic to use for cross-compiled components, but it would give users a safe, honest and easy to understand, if slightly cumbersome to use, approach to writing cross compiled components.

Other Scala.js UI libraries may face less of an issue running on the JVM as they use platform-agnostic virtual DOM abstractions, and use reactive systems that were designed for the backend, but Laminar and Airstream take different tradeoffs, optimizing for ergonomics of frontend SPA development, so the cross compiling support will inevitably have more friction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants