wasp icon indicating copy to clipboard operation
wasp copied to clipboard

Add tracking of wasp version to the Wasp project

Open Martinsos opened this issue 3 years ago • 1 comments

Idea: what if we had a field in .wasp that specifies which Wasp version is used for building specific project? I think at some point we will want to have that for sure, right? So when you want to run a Wasp project, you are sure you are not using wrong Wasp compiler version. It should be enough to just specify major.minor version -> then we know that it has to be that major version, and minor version has to be bigger then specific minor version. So basically if we have 3.2 in the file, we will allow 3.3, 3.4 and similar, but won't allow 4.0 or anything above that. This assumes you won't ever want single Wasp project to support being built with different major versions, which I think it is ok assumption for now. We could say format has to be waspVersion: ^major.minor or waspVersion: ^0.major.minor to also support alpha/beta releases, and ^ is there to makeit more clear how wasp handles this version + to possibly make it easier to add more freedom in the future.

I would imagine this field would be required, so you can't have Wasp project for which it is not clear which Wasp version needs to be used. And if your wasp CLI doesn't match the version of the project, it would throw an error.

Field could be app.waspVersion, or maybe wasp.version (with wasp being new declaration for wasp specific config. In theory it could even be a separate file, but it seems weird to have it in separate file when we have .wasp.

Martinsos avatar Apr 21 '22 13:04 Martinsos

Some initial direction pointers:

  1. Look into AppSpec/ and extend the AppSpec so it has waspVersion in it.
  2. In Wasp compilation step (so somewhere in Analyzer probably, or close to it), we would want to check if Wasp version matches the version in waspVersion. I am not 100% sure if we would want to do that in cli/ or in src/, but we do smth for start and then see from there.

Martinsos avatar Sep 20 '22 18:09 Martinsos

Hello! I would like to work on this issue. I was already checking the code and I think I have a good idea of where to start :smile:

What do you think the error message should be? I was thinking of something like "Your Wasp version (a.b.c) should match ^x.y.z"

neolight1010 avatar Oct 20 '22 03:10 neolight1010

Hey @NeoLight1010 ! Sure, go for it! The error message sounds fine, maybe we can tweak it a bit in the direction of "Current Wasp project requires Wasp version ... but you are using ...".

Before you get too deep into implementation, I would maybe advise you to check with me on how you would approach it -> so if you could explain to me shortly in which pieces of code would you make changes and how it would work, we can avoid bigger corrections later. You can just write it here! And take into account what I wrote in description -> there is also a question of where we specify the version, is it in main.wasp file, or in some other file, or what.

Martinsos avatar Oct 20 '22 17:10 Martinsos

Thank you for assigning the issue to me :D

About the implementation, I think the function validateAppSpec would be a good place to validate the compiler's version, as that's where most other field-specific validation is:

https://github.com/wasp-lang/wasp/blob/dc036f56cdc31a43a1d84fe96f7add174c34fd8b/waspc/src/Wasp/AppSpec/Valid.hs#L29-L40

I also thought about doing it in analyzeWaspProject or Analyzer.analyze, but that seems more related to "parsing" and converting the .wasp file's contents to AppSpec, so I don't think the validation belongs there:

https://github.com/wasp-lang/wasp/blob/dc036f56cdc31a43a1d84fe96f7add174c34fd8b/waspc/src/Wasp/Lib.hs#L58-L97

About where the version field should go, I think it would be best to have it as wasp.version. That makes more sense to me and would allow for more compiler-specific configuration in the future.

I am still not sure about the format of the field, though. Maybe just use x.y.z for now to keep things simple, and later add the SemVer specific tokens like ^ and ~?

Let me know what you think.

neolight1010 avatar Oct 21 '22 01:10 neolight1010

@NeoLight1010 nice analysis!

I am in with the idea of having smth like

wasp {
  version: "^1.2.3"
}

in AppSpec, that makes sense to me, and as you said opens some space for further configuration. If needed, we can in the future take it out, but for now this sounds ok. We would make that required field.

I would maybe quickly check with the rest of team here though @shayneczyzewski @sodic @matijaSos , what do you think?

As for where we would analyze it -> Yup, I think AppSpec Validation step is good!

Finally, version format: I would propose going with what I suggested in the initial comment on this issue, which is ^x.y.z or ^0.x.y.z. Then later we can possibly expand on that if needed. Do notice that we have a SemVer module in our codebase that knows how to work very nicely with semver, so that one might come in handy!

Martinsos avatar Oct 21 '22 15:10 Martinsos

After some discussion on Discord, we agreed to use app.wasp.version instead of wasp.version, as the current implementation doesn't allow unnamed entries in the .wasp file, and adding support for that would require a lot of knowledge of the codebase.

neolight1010 avatar Oct 24 '22 04:10 neolight1010

All this sounds good @Martinsos and @NeoLight1010 I think doing something like app.wasp.version until we can pull it out top makes sense. 👍🏻

One other thing to keep in mind is our installer/CLI doesn't make it super clear how to install a specific version. I wonder if we should provide a snippet/info/URL for them to install the specific version they need in our error message. Over time I think our CLI will make handling multiple versions easier, but that is lacking right now. Even if this requires a bit of tweaking to the installer script, it may be worth it for near-term DX if we make this a new required field. Just a thought. Excited about this! 🥳

shayneczyzewski avatar Oct 24 '22 12:10 shayneczyzewski

Sounds good @shayneczyzewski . Where should this message lead to? I can't find any info in the docs for how to install a specific version. Maybe the GitHub Releases page?

neolight1010 avatar Oct 25 '22 02:10 neolight1010

Just a couple of things to add here.

Syntax considerations

I think I'd prefer the following ways of defining the version:

app Something {
    version: "^1.2.3"
    ...
}

Or maybe:

app Something {
    waspVersion: "^1.2.3"
    ...
}

Or even:

app Something {
    wasp: "^1.2.3"
    ...
}

Instead of a nested block (looks a bit excessive):

app Something {
    wasp: {
        version: "^1.2.3"
    }
    ...
}

What do you guys think?

One possible caveat

@Martinsos Keep in mind that our SemVer module does not support all Node semver version specifiers. Users might expect being able to specify something like ~1.2.3. This won't work. I don't think that's a problem for now (no one will be specifying Wasp versions anyway), it's just something to make a mental note of.

The error message and where it leads to

You can follow the style of our Node version mismatch message.

Sounds good @shayneczyzewski . Where should this message lead to? I can't find any info in the docs for how to install a specific version. Maybe the GitHub Releases page?

Ah yes, this is gonna be a problem :sweat_smile:. I'll let @Martinsos answer this one.

sodic avatar Oct 25 '22 09:10 sodic

Sounds good @shayneczyzewski . Where should this message lead to? I can't find any info in the docs for how to install a specific version. Maybe the GitHub Releases page?

I agree with @sodic this may be better answered by @Martinsos. I'm not sure if we will need to update the installer script, or if we can provide a snippet where they can easily install from a release link. Maybe we can just add a section to the docs for how to manually do so (once we test it all out) in the worst case and just point them there?

shayneczyzewski avatar Oct 25 '22 12:10 shayneczyzewski

@sodic About the specific syntax, I think app.wasp.version does look a bit verbose for now, but I think it would be useful for when more compiler options might are added.

About the SemVer module limitations, we could make it clear in the docs and in the error message that the only allowed syntax for now would be "^0.major.minor.patch", until the SemVer module is changed to allow the other tokens.

Let me know what you think :D

neolight1010 avatar Oct 25 '22 22:10 neolight1010

Nice brainstorming happening here!

  1. app.wasp.version vs app.waspVersion vs app.wasp ... I don't think it matter greatly at the moment. I think choice is between app.wasp.version and app.waspVersion, others don't have great naming. Of those two, I would maybe go with the nested one, to leave some space for future configurations here. But truth is I don't know what those are, and this is probably going to change anyway, so I am good with any of these two options. I suggest @NeoLight1010 you pick smth and we go with it.
  2. Limitations on the version -> idea, as discussed above, is that for now we support only ^0.major.minor.patch, as @NeoLight1010 mentioned. Or something in that direction. But it is ok that for now we support a limited subset of versions, just to get us started, we can later expand that.
  3. Actually installing the newer version -> ah that is a tricky one as @shayneczyzewski said :D. Actually not really a tricky one, it is just that we don't have support for that yet! I think the best way to add support for it is to extend the installer script to support installing a specific version. Installer script is here: https://github.com/wasp-lang/get-wasp/blob/master/installer.sh . So we could extend it to take one extra argument, for example -v 0.1.2.3 which will make it install that exact version (right now it installs the latest version always). We will need to make a separate PR for this since it is in another repo. @NeoLight1010 you can also do that if you want, or we can leave that to someone else if it is bit too much right now.

Martinsos avatar Oct 26 '22 09:10 Martinsos

Cool! @Martinsos Then I think we can agree on:

  1. app.wasp.version, because I already had this one partially done :smile: I can change it to app.waspVersion if you think that's better.
  2. Supporting only ^0.major.minor.patch for now.
  3. Yes, I would love to work on that feature for the installer :D I can do it after I finish this issue, if that's ok.

Thank you, guys!

neolight1010 avatar Oct 26 '22 19:10 neolight1010

Sounds good @NeoLight1010 let's do it :)!

Martinsos avatar Oct 26 '22 21:10 Martinsos