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

rfc(374): The jsii compiler to follow TypeScript versioning #373

Merged
merged 29 commits into from
Oct 13, 2023

Conversation

RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Aug 23, 2021

This RFC #374 proposes to change the versioning scheme of the jsii compiler to stop
conforming to semantic versioning, and instead use the major.minor
version of the TypeScript compiler it is built on.


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache-2.0 license

@RomainMuller RomainMuller self-assigned this Aug 23, 2021
This PR proposes a new way to model the `aws/jsii` project in order to
alleviate friction in making significant changes to the platform,
without compromising on key elements of the developer experience, or
risking to unnecessarily fragment the ecosystem.
@RomainMuller RomainMuller changed the title feat: jsii modular versioning rfc(374): jsii modular versioning Aug 23, 2021
@RomainMuller RomainMuller marked this pull request as ready for review August 23, 2021 12:29
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
text/0374-jsii-modular.md Outdated Show resolved Hide resolved
Comment on lines 167 to 169
maintain backwards compatibility between individual packages, and reduce the
risk of creating codependent packages (different repositories need to be
released independenlty, so dependencies can only flow in an asyclic manner).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't monorepos provide the same or some of these assurances? For eg, I believe you can't have cyclic dependencies within packages of a monorepo.

It's not clear to me (from the rfc) why coupling jsii versions and breaking off monorepo are related.

By not breaking off the monorepo, some of the concerns you've listed in the next question can be avoided while still decoupling jsii release versions.

text/0374-jsii-modular.md Outdated Show resolved Hide resolved
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am missing a deeper discussion on compatibility boundaries between the tools.

  • The .jsii manifest is a boundary between the compiler and pacmak (and jsii-relect) (do we really need those two to be separate tools? Maybe we can merge them into a single tool).
  • There is also a compatibility boundary between the runtime and the generated code. If two modules use different versions of pacmak, do they need two runtimes?

In my mind having a formal compatibility model is the first thing we must finalize before we are able to clearly break this coupling.

On the other hand, there are some low-hanging fruit that we can easily extract from the main repo (e.g. superchain).

text/0374-jsii-modular.md Outdated Show resolved Hide resolved
@RomainMuller RomainMuller changed the title rfc(374): jsii modular versioning rfc(374): The jsii compiler to follow TypeScript versioning May 3, 2022
@RomainMuller RomainMuller requested a review from a team May 3, 2022 12:12
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Show resolved Hide resolved
text/0374-jsii-ts-version.md Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
Comment on lines +214 to +217
* **Optional:** Separate the `jsii` compiler from the rest of packages in the
jsii toolchain into separate repositories. This would make it easier to
release the `jsii` compiler separately from other parts of the toolchain that
follow a different versioning scheme.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible projen use case?

Comment on lines +256 to +261
This is not a breaking change in the traditional sense, however this is a
significant change in versioning schemes used for the `jsii` compiler, and a
departure from the [AWS SDKs and Tools maintenance policy][aws-policy].

As such, this warrants ample communication with our customer base to avoid they
are caught by surprise, or have broken expectations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be clear: it either is a breaking change or it isn't. If customers have to change their code (I think they might have to since newer TypeScript versions introduce breaking changes) then it's a breaking change, I think. If any migration does not require customers to change any of their code, then it's not a breaking change. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I want to say here is that we are making a breaking change... to our versioning scheme (SemVer -> TypeScript-Ver).

Customers will have to update their code when they move from jsii@1.x to jsii@4.7.x due to language specification changes, but the real think they need to be very carful about is to update from a caret dependency jsii@^1.0.0 to a tilde dependency jsii@~4.7.0.

The level/duration of maintenance for future versions will also be different, and we should definitely encourage users to closely track latest as much as possible.

text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Show resolved Hide resolved
text/0374-jsii-ts-version.md Show resolved Hide resolved
RomainMuller and others added 2 commits September 26, 2022 10:19
Co-authored-by: Momo Kornher <mail@moritzkornher.de>
Co-authored-by: Momo Kornher <mail@moritzkornher.de>
rix0rrr
rix0rrr previously approved these changes Oct 11, 2022
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
Comment on lines 39 to 40
others) are not affected by this change and will continue to follow [semantic
versioning][semver] as they currently do. The bindings generated by
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this document say anything about the backwards compatibility guarantees of these tools?

For example, we could be MV-bumping on every release, and proudly claim "we are following semver", yet our customers would still hate us.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does in the sense that it has provisions around how you shouldn't be forced to upgrade your dependency on a jsii tool because one of your dependencies or dependents has upgraded already (so every new line must be able to operate on output of previous AND future versions).

text/0374-jsii-ts-version.md Show resolved Hide resolved
currently active [TypeScript] release line.

Critical bug fixes and security patches will be provided for previous release
lines for 12 months, which covers 3 or 4 previous lines. Features will not be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current state of things, bug-and-security maintenance for a compiler line does not require a ton of work (few bug reports against the compiler), and so I am assuming 1 year of maintenance is not an issue.

That said, this is a line in the sand and we can absolutely discuss where it should be put.

versioning][semver], this implies `jsii` and `jsii-rosetta` also need to stop
conforming to it.

### Why should we _not_ do this?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another "why should we not do this": customers will need to potentially change their code to get access to bug fixes and features in new jsii releases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to get access to bug fixes

Only if they are currently on a compiler release that is end-of-life, which... I guess is the same with all end-of-life software.

and features

This is by design!

Copy link
Contributor

@rix0rrr rix0rrr Oct 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if they are currently on a compiler release that is end-of-life, which... I guess is the same with all end-of-life software.

I might have missed this, but are you backporting all bugfixes to all versions?

and features

This is by design!

Well. Yes for TSC features, but also for jsii-related features? Let's say we add a "named type unions" feature to jsii... would that feature also be backported to all 4 current release lines? Or just be added to the latest?

That's quite a heavy maintenance burden you would be taking on if yes...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have missed this, but are you backporting all bugfixes to all versions?

There is a whole section on the maintenance policy that states we will fix bugs in releases for 1 year after they cease to be "current" (I have added alternate scenarios there -- one for 6 months instead of 1 year, and one for "only current + previous" which is what Go does). Sooo technically not "all versions", but "some" for sure.


Let's say we add a "named type unions" feature to jsii...

That is certainly an interesting case, because such a thing does not exist in the current type model... We'd either need to design a strategy that allows us to add that feature in a strictly backwards compatible manner (sounds hard if even possible w/o voiding the feature entirely), or we might need to back-port that feature on non-EOL compilers (and yes - in this case that sounds like more work than I'd like to sign up for, if we uphold the 12 months policy).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a blurb in "why not" about this as you suggested. Not sure this is a deal breaker here, but certainly should inform our final decision on the maintenance/support policy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer we didn't backport changes like that for a whole year. It was a ton of maintenance work for the oncall when we did it for CDK...it made the oncall's life much easier when CDK v1 finally hit EOL.

Copy link
Contributor

@ansgarm ansgarm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some tiny typos while reading through your RFC, @RomainMuller 🔍😄

I really like this proposal!
It was interesting to read as matching a TypeScript version with JSIIs major.minor sounds like a solution to a quite similar problem we're facing with pre-built provider bindings in CDKTF which depend on the CDKTF version and the underlying Terraform Provider version (also TIL about cdk8s approach here).

text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
RomainMuller and others added 3 commits October 25, 2022 10:09
Co-authored-by: Ansgar Mertens <mertens.ansgar@gmail.com>
Co-authored-by: Ansgar Mertens <mertens.ansgar@gmail.com>
Co-authored-by: Ansgar Mertens <mertens.ansgar@gmail.com>
@mergify mergify bot dismissed rix0rrr’s stale review October 25, 2022 08:10

Pull request has been modified.

rix0rrr
rix0rrr previously approved these changes Oct 25, 2022
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still good with this plan if you are. Just making sure we're going into all consequences eyes open :).

text/0374-jsii-ts-version.md Outdated Show resolved Hide resolved
versioning][semver], this implies `jsii` and `jsii-rosetta` also need to stop
conforming to it.

### Why should we _not_ do this?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer we didn't backport changes like that for a whole year. It was a ton of maintenance work for the oncall when we did it for CDK...it made the oncall's life much easier when CDK v1 finally hit EOL.

line, including a planned timeline for the initial release of the `v4.9` line,
and language about the departure from [SemVer].

* **Optional:** Separate the `jsii` compiler from the rest of packages in the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is worthwhile. I think it's confusing if jsii follows different versioning than jsii-pacmak when they live in the same repo.


- Make the `github.com/aws/jsii` repository by a single-package repository.

* Update the *release* automation to use the [TypeScript] compiler version's
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean that new jsii versions can only be made when there are new TSC releases? Should we add a new dot to the release for any critical regression fixes, so we have a

major.minor.patch.jsiiPatch scheme?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure npm will not accept that version scheme.

Concretely, we are free to make new versions whenever we want under the new scheme, but they'll all be "patch" versions (introduction of new features, bug fixes, etc... all those as patch versions).

We can however only make BREAKING changes in a version when a new TypeScript compiler version is released (this happens once per quarter, which I believe is frequent enough).

Co-authored-by: Calvin Combs <66279577+comcalvi@users.noreply.github.com>
Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was accepted

@mrgrain mrgrain merged commit ea656c3 into main Oct 13, 2023
2 checks passed
@mrgrain mrgrain deleted the rmuller/jsii-version-unlock branch October 13, 2023 17:59
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.

8 participants