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

[VSCode] server-emitter use floating version for the peer-dependenies #5385

Open
chunyu3 opened this issue Dec 17, 2024 · 10 comments
Open

[VSCode] server-emitter use floating version for the peer-dependenies #5385

chunyu3 opened this issue Dec 17, 2024 · 10 comments

Comments

@chunyu3
Copy link
Contributor

chunyu3 commented Dec 17, 2024

E2E VSCode extension which will include client code generation and server code generation. E2E VSCode extension will install needed emitter packages if it is not installed before.

But now, we may meet with dependency version conflict issue.
e.g. First there is an old @typespec/http-server-csharp@0.58.0-alpha.3 was installed, and it will dependent on @typespec/compiler@0.60.0.
Then we will help to install @typespec/http-client-csharp package, and it will accept @typespec/compiler (>=0.63, <1.0.0), so when we install @typespec/http-client-csharp there will be peer-dependency conflict with @typespec/http-server-csharp@0.58.0-alpha.3.
Image

Option 1: workaround
To resolve the issue, the workaround is that we need to upgrade all existing emitter packages (client and server emitter) to latest version. This is a workaround, and it is not good customer experience: customer may not want to upgrade all the emitters when generate a code.

Option 2:
Use the floating version in peer-dependency.

As the client code emitter use peer-dependency with a floating version as following:

Image

but the server emitter does not use floating version, see

Image

Can server emitter has the same floating version as client emitter? In that case, we can resolve the peer dependency conflict when there are both client emitters and server emitters?

@chunyu3 chunyu3 changed the title [VSCode] server-emitter use float version for the peer-dependenies [VSCode] server-emitter use floating version for the peer-dependenies Dec 17, 2024
@allenjzhang
Copy link
Member

Option 2 makes sense to me. @markcowl

@mikeharder
Copy link
Contributor

mikeharder commented Dec 19, 2024

Please include me in any conversations about package or dependency versions. I'd need to understand the broader context, but we should be able to configure things to allow mixing versions (or require matching versions) as we see fit.

Things may also look different pre-1.0 and post-1.0, depending how closely we decide to follow semver. Pre-1.0, NPM makes it more difficult to allow version ranges.

Our general approach so far, has been to allow broad ranges of versions at install time, so things will either just work (if no breaks) or fail at runtime. Post-1.0, we may want to offer stronger compat guarantees.

So right now, I agree http-server-csharp should match http-client-csharp. But I also want to think about our longer-term and bigger-picture versioning story.

@chunyu3
Copy link
Contributor Author

chunyu3 commented Jan 3, 2025

hello @allenjzhang @markcowl The same issue will occur in openapi emitter @typespec/openapi and @typespec/openapi3, @typespec/json-schema, @typespec/protobuf

@markcowl markcowl self-assigned this Jan 6, 2025
@markcowl markcowl added this to the [2025] January milestone Jan 6, 2025
@markcowl markcowl added emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp feature New feature or request labels Jan 6, 2025
@timotheeguerin
Copy link
Member

Writing here what I said in the meeting, the http-server-csharp emitter is part of the workspace and follow the same release schedule as all other packages there. It just doesn't follow the same pinned version(0.64.x).
This means that it doesn't need to have those floating versiom in the same way the openapi3, protobuf and all our other emitters/libraries don't. You upgrade everything at once.

For the csharp-client emitter they release on a separate schedule which means they need to be compatible with at least 2 set of version to not have a period where we are stuck. We decided to have this very wide floating just for convinience

@RodgeFu
Copy link
Contributor

RodgeFu commented Jan 27, 2025

I think our emitter's version policy shouldn't be decided by whether it's developed or released along with compiler, but whether it makes sense to force our end user to always use the specific versions of these emitters and compilers together and the impact on user experience from it. The coupling between these emitters and compiler are not necessary to me which would actually cause unnecessary bad user experience and trouble considering they are naturally different concept in TypeSpec and user would have different expectation when using them. Some scenarios I can think about:

  1. When we force these emitter's version must equal to a specific compiler, it means for any user who wants to install the new emitter by 'npm install {emitter}' would 100% fail for the first try if the compiler(or other emitters) in his TypeSpec project isn't the latest (you can easily repro it by 'npm install @typespec/compiler@0.63.0' and 'npm install @typespec/http-server-csharp'). I know user can look into the error message and upgrade and try again..., but the 100% fail rate for the first try itself is already a very bad user experience in my opinion especially when it's not an uncommon scenario for people having not-latest compile (or other emitters) in the project. Not saying one of our major target customers is from openapi who may not familiar with npm which would be more painful for them to figure out what's going on (I don't think npm error message is so friendly for people who's new to it).

  2. After GA, I believe many customers would start to use TypeSpec in their PROD environment (which is also what we want). From my experience in Azure team, people won't want to upgrade so frequently after things going to PROD especially for those big enterprises because any failure in PROD is a big incident to them. So usually, they would need to plan for resource to test the new version thoroughly before they are comfortable to deploy the new version to their PROD (As you may know, the planning in big enterprise itself may take weeks.). And let's say some company tried TypeSpec and likes it, so they used TypeSpec to define and emit some code and deployed to their PROD. Months later they got new requirement and want to generate other languages code. Then he suddenly finds installing the new emitter just doesn't work. He needs either upgrade the compiler and other installed emitters or figure out the correct old version emitter matching the compiler version they are using. I think neither of them are good to end user which would definitely delay their plan and the effort to use TypeSpec more. And if we think more, they may even question the compatibility of TypeSpec which would be an item in their cons. list when evaluating what to use next time when planning other similar project.

So in my opinion, the version policy of emitter should just depend on the backward compatibility our compiler can provide and whether emitter needs new support/feature from compiler so that we can provide the best user experience by avoiding unnecessary trouble of version confliction which is one of the most headache problem to end user. thanks.

@timotheeguerin
Copy link
Member

timotheeguerin commented Jan 27, 2025

So there is 2 part to separate, now and after 1.0. Now we do not garantee 100% back compat between minor version so we do this as the only version we can garantee it will work together are teh ones with the same major.minor. This is pretty much the same for the client emitters that use their "floating" versioning expect they say their version willl work with any following version of the compiler and libraries(which is not necessarly true but helps with installing dependency, but also wouldn't solve your scenario of adding this emiter at a later point in time).

Now after v1, I am still not sure I agree, it is very simple right now to ensure all your dependency match the same version(on top the testing is basically impossible to verify as we have all packages there and would create a massive test matrix), typespec involve a lot of dependencies and it was a nightmare before to try to find which ones works together which will inevitable be the case sa certain emitters/libraries will need newer features.
Having the whole set of libraries share teh same version is a very common pattern used across similar ecosystem. As far as I can tell aspnet does the same https://www.nuget.org/packages/Microsoft.AspNetCore.Http.Abstractions#dependencies-body-tab

@RodgeFu
Copy link
Contributor

RodgeFu commented Jan 29, 2025

Yeah, I think it's fine before 1.0. But after GA, people would have higher expectation to us especially in stability and compatibility. And also they will start using our Prod in bigger and more complex scenario with longer life cycle. So simply coupling our compiler and the emitter/libs may bring more trouble in some case like I mentioned above. (Especially that the compiler is in the core position of TypeSpec, so the potential impact from changing/updating it may not be ignorable or trivial in some case tech or non-tech)

For your concern of the massive test matrix, do you have more detail? Because

  • From compiler perspective, I think we need to provide backward compatibility anyway which is a must-have for people to start using TypeSpec seriously.
  • From emitter/lib perspective, I think the compiler should be treated as normal dependency just like other dependencies, isn't it? Just like if we are developing these emitter/lib not in compiler workspace, how would we do the test? I guess the test should focus on their features and their own compatibility (which is also a must-have) while trusting compiler and other dependencies do a good job in compatibility. (Otherwise, what's our expectation for our user to do the test when they develop custom emitters or libraries of TypeSpec?)
    Of course, it's better for us to also have some e2e test to cover complex scenario with multiple emitter/lib included, but I think its focus may should be on some key combination instead of a full combination considering we should do a good job on the two bullets above.

As for the nightmare you mentioned, I don't have much context but I can image and had similar experience in other project in the early stage when everything is evolving/changing quickly. But I think things are different now considering we are GA soon. If we still have the concern that our compiler, emitters and libraries may not work well with each other and can only guarantee their latest version to work even now, then I would have some concern and would strongly suggest that we should spend some time before GA to sort out their relationship and the compatibility we can provide for GA.

As for the common pattern of ecosystem, to be honest, I think I am still new to the open source world (I mostly work on C#/Sql/Azure before). But from my experience on developing, it feels not that common to me that I have to upgrade my compiler in order to use a library no matter when I am using .Net or other languages. thanks.

@timotheeguerin
Copy link
Member

Yes, after v1 the general rule is you should be able to use the followin pattern ^1.x.z to depend on the compiler and our libraries as long as you do not use things that we consider internal or advanced usage(#4914). And the same technically applies to our libraries as well. You are however talking about some forward compatibility not back compat. If you add an emitter later in time there will never be and should never be a requirement that this latest emitter version works with every published version of the compiler and its libraries. It might need new features added later.

For the testing part for best practice a library should at least test with the oldest compatible version they support(as you assume the next minor version will be back compat) there is case where you might want/meed to also test a range of version which can be manageable when you work on a single library. However we have dozen which could(if we stop requiring using the same version or above) require us to not only run the workspace test but for each package install the extra dependency and do it again. Creating a n * m amount of test you might need to run.

My opinion is that when we get to v1 we change maybe to this version requirement ^1.{currentMinor}.0 which provide the simplest solution for managing everything for us and the user and would be accuratly following our breaking change policy(as long as we actually use semver and not typescript like versioning in which case we have to keep it as it is). If a user wants to add a package later and don't want to upgrade then they can just easily add teh same version they are already using.

I think you put too much emphasis on upgrading the compiler being different from upgrading libraries(where even if that was the case this still doesn't solve anything as you just push the problem to the first layer of libraries), all of those are basically the same in the typespec world.

@RodgeFu
Copy link
Contributor

RodgeFu commented Jan 31, 2025

Maybe I didn't express myself clearly above. I didn't mean forward compatibility. What I mean is something like ^1.{minor}.0 where {minor} will keep as it is until the emitter/lib needs new feature from compiler or we want to push our users to upgrade to some compiler version for some reason (i.e. we don't want end user to use too old compiler). But the ^1.{currentMinor}.0 you mentioned is also a way to manage versions which has a smaller impact to us while resolving some problem to some extent. I am fine to start from it and see whether we will get further feedback from our end users. thx.

@markcowl markcowl modified the milestones: [2025] February, [2025] March Feb 6, 2025
@markcowl markcowl added emitter:service:csharp 1_0_E2E and removed emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp labels Feb 13, 2025
@markcowl
Copy link
Contributor

markcowl commented Mar 3, 2025

At this point, we will have a discussion around dependencies and ensure that http-server-csharp uses the suggested pattern for emitters.

@markcowl markcowl modified the milestones: [2025] March, [2025] April Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants