feat(jsii): experimental --strip-deprecated feature #2437

RomainMuller · 2021-01-19T17:06:00Z

This feature will cause jsii to erase all declarations marked with
@deprecated from the .jsii assembly and .d.ts files.

When a @deprecated type is extended (or implemented) by a type that
is not @deprecated, the inheritance chain will be corrected, erasing
the @deprecated type and replacing it with it's non @deprecated
parents, if any.

Remaining uses of @deprecated members will be reported through
diagnostic messages (with the ERROR severity).

A new module bindings was implemented to allow relating TypeScript AST
nodes to "arbitrary" points in the jsii assembly, allowing to recover
that information once the tree was fully processed. This makes
multi-pass transformations not need to re-inspect the whole AST again.

⚠️ This is currently unable to identify interface implementations that become
invalid (in the type definitions) as a result of erasing a base class.

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

This feature will cause `jsii` to erase all declarations marked with `@deprecated` from the `.jsii` assembly and `.d.ts` files. When a `@deprecated` type is extended (or implemented) by a type that is not `@deprecated`, the inheritance chain will be corrected, erasing the `@deprecated` type and replacing it with it's non `@deprecated` parents, if any. Remaining uses of `@deprecated` members will be reported through diagnostic messages (with the `ERROR` severity). --- A new module `bindings` was implemented to allow relating TypeScript AST nodes to "arbitrary" points in the jsii assembly, allowing to recover that information once the tree was fully processed. This makes multi-pass transformations not need to re-inspect the whole AST again.

nija-at

Architectural qn (non blocking): we have a couple of transformers here now - this one and the one for experimental API docs. How far away do you think we are to making these PnP to the jsii core compiler (not necessarily to the CLI or configuration)?

nija-at · 2021-01-20T12:21:40Z

packages/jsii/lib/transforms/deprecated-remover.ts

@@ -0,0 +1,891 @@
+import {


I would recommend tests for such a large and complex code. I suppose it's coming soon.

...granted I am not allocated to another project...

RomainMuller · 2021-01-20T12:39:25Z

Architectural qn (non blocking): we have a couple of transformers here now - this one and the one for experimental API docs. How far away do you think we are to making these PnP to the jsii core compiler (not necessarily to the CLI or configuration)?

To be honest - I very much dislike the current state of this; and it'd likely deserve a major overhaul. The main jsii business needs to happen as a transform, too, so we don't have to actually jump though the kind of hoops we are currently... The performance right now would be hit by the fact we actually cause the TypeScript compiler itself to emit (i.e: write output files, like .js & .d.ts files) at least twice, maybe thrice... This is...... unnecessary.

nija-at

Provisional approval

nija-at · 2021-01-21T09:28:59Z

packages/jsii/bin/jsii.ts

@@ -54,6 +54,12 @@ const warningTypes = Object.keys(enabledWarnings);
            desc: `List of warnings to silence (warnings: ${warningTypes.join(
              ',',
            )})`,
+          })
+          .option('strip-deprecated', {


Expand and document this in the jsii docs. Also explain any pre-requisites, such as deprecated types must not be used in such-and-such places, etc.

The whole point of it being EXPERIMENTAL here is I actually don't want to publicise it too much just yet...

Maybe add an experimental section to the docs 🤷‍♂️
Fine with me if you feel strongly

packages/jsii/test/deprecated-remover.test.ts

nija-at · 2021-01-21T09:32:22Z

packages/jsii/test/deprecated-remover.test.ts

+
+const DEPRECATED = '/** @deprecated stripped */';
+
+test('produces correct output', async () => {


Would've been nicer to organize these fixtures and expectations as actual folders with files.
Would be much easier to add new cases as we fix or add features to the remove-deprecated.

WDYT?

Expectations as actual folders with files means we have to write our own way to update snapshots (jest -u will only update .ts.snap files, and the structure there is not very flexible). Also - how is more files to know about and modify, better than the whole GIVEN/WHEN/THEN is in a single place?

nija-at · 2021-01-21T09:38:15Z

packages/jsii/bin/jsii.ts

@@ -54,6 +54,12 @@ const warningTypes = Object.keys(enabledWarnings);
            desc: `List of warnings to silence (warnings: ${warningTypes.join(
              ',',
            )})`,
+          })
+          .option('strip-deprecated', {


Is there a way to ensure that when 'new' jsii features are supported (say, union types), they are correctly handled by this flag?

Not expecting it in this PR, but wondering if you've thought about this.

One way that comes to mind is - "some" automated mechanism (hand wave) of taking the jsii fixture (calc) and marking most or all of them as deprecated and stripping them out.

Another option - if there's some place where we've centralized the jsii specification and supported features, it might be easier walk through them and dynamically generate a fixture and run them through strip-deprecated.

(Hand wave) Contributions are welcome 🤓

nija-at · 2021-01-21T09:44:04Z

packages/jsii/bin/jsii.ts

@@ -54,6 +54,12 @@ const warningTypes = Object.keys(enabledWarnings);
            desc: `List of warnings to silence (warnings: ${warningTypes.join(
              ',',
            )})`,
+          })
+          .option('strip-deprecated', {


(Not necessary to do it in this PR)

What do you think about adding this option to the jsii component of package.json as well?

I'm favorable to this but did not have time to go about it... Should be easy enough to do.

mergify · 2021-01-26T17:39:32Z

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

mergify · 2021-01-26T17:40:08Z

Merging (with squash)...

aws-cdk-automation · 2021-01-26T19:33:38Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-5lHf64IXfvmr
Commit ID: a598334
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2021-01-26T20:37:44Z

Merging (with squash)...

RomainMuller requested a review from nija-at January 19, 2021 17:06

RomainMuller self-assigned this Jan 19, 2021

mergify bot added the contribution/core This is a PR that came from AWS. label Jan 19, 2021

RomainMuller added 2 commits January 20, 2021 10:12

fix compilation error

11f0c0a

re-work export declarations, too

59dadf8

RomainMuller marked this pull request as ready for review January 20, 2021 11:41

Merge branch 'main' into rmuller/hide-deprecated

4db526c

nija-at reviewed Jan 20, 2021

View reviewed changes

RomainMuller added 2 commits January 20, 2021 13:46

fix compiler tests

76bc976

add test for deprecated remover output

79b7dea

nija-at added the pr/do-not-merge This PR should not be merged at this time. label Jan 21, 2021

nija-at approved these changes Jan 21, 2021

View reviewed changes

RomainMuller added 2 commits January 25, 2021 13:56

Merge branch 'main' into rmuller/hide-deprecated

46338f5

add doc for experimental flag

924c7b6

RomainMuller removed the pr/do-not-merge This PR should not be merged at this time. label Jan 25, 2021

Merge branch 'main' into rmuller/hide-deprecated

9258036

mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Jan 26, 2021

Merge branch 'main' into rmuller/hide-deprecated

f8cf858

mergify bot added 2 commits January 26, 2021 17:58

Merge branch 'main' into rmuller/hide-deprecated

d4645e0

Merge branch 'main' into rmuller/hide-deprecated

a598334

mergify bot merged commit f958f5a into main Jan 26, 2021

mergify bot deleted the rmuller/hide-deprecated branch January 26, 2021 20:37

mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Jan 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(jsii): experimental --strip-deprecated feature #2437

feat(jsii): experimental --strip-deprecated feature #2437

RomainMuller commented Jan 19, 2021 •

edited

Loading

nija-at left a comment •

edited

Loading

nija-at Jan 20, 2021

RomainMuller Jan 20, 2021

RomainMuller commented Jan 20, 2021

nija-at left a comment

nija-at Jan 21, 2021

RomainMuller Jan 21, 2021

nija-at Jan 22, 2021

nija-at Jan 21, 2021

RomainMuller Jan 25, 2021

nija-at Jan 21, 2021

RomainMuller Jan 25, 2021

nija-at Jan 21, 2021

RomainMuller Jan 25, 2021

mergify bot commented Jan 26, 2021

mergify bot commented Jan 26, 2021

aws-cdk-automation commented Jan 26, 2021

mergify bot commented Jan 26, 2021


		const DEPRECATED = '/** @deprecated stripped */';

		test('produces correct output', async () => {

feat(jsii): experimental --strip-deprecated feature #2437

feat(jsii): experimental --strip-deprecated feature #2437

Conversation

RomainMuller commented Jan 19, 2021 • edited Loading

nija-at left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RomainMuller commented Jan 20, 2021

nija-at left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Jan 26, 2021

mergify bot commented Jan 26, 2021

aws-cdk-automation commented Jan 26, 2021

AWS CodeBuild CI Report

mergify bot commented Jan 26, 2021

RomainMuller commented Jan 19, 2021 •

edited

Loading

nija-at left a comment •

edited

Loading