Skip to content

Commit

Permalink
fix(jsii): require statement for the warning file is generated when i…
Browse files Browse the repository at this point in the history
…t's not used (#3184)

When `add-deprecation-warnings` is `true` but no calls were injected in a given file, the `require` statement for `.warnings.jsii.js` should not be generated.


---

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

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
otaviomacedo authored and Niranjan Jayakar committed Nov 18, 2021
1 parent bf38a08 commit 8f53f89
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
14 changes: 13 additions & 1 deletion packages/jsii/lib/transforms/deprecation-warnings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,8 @@ module.exports.DeprecationError = DeprecationError;
}

class Transformer {
private warningCallsWereInjected = false;

public constructor(
private readonly typeChecker: ts.TypeChecker,
private readonly context: ts.TransformationContext,
Expand All @@ -342,9 +344,11 @@ class Transformer {
) {}

public transform<T extends ts.Node>(node: T): T {
this.warningCallsWereInjected = false;

const result = this.visitEachChild(node);

if (ts.isSourceFile(result)) {
if (ts.isSourceFile(result) && this.warningCallsWereInjected) {
const importDir = path.relative(
path.dirname(result.fileName),
this.projectRoot,
Expand All @@ -368,6 +372,8 @@ class Transformer {
private visitor<T extends ts.Node>(node: T): ts.VisitResult<T> {
if (ts.isMethodDeclaration(node) && node.body != null) {
const statements = this.getStatementsForDeclaration(node);
this.warningCallsWereInjected =
this.warningCallsWereInjected || statements.length > 0;
return ts.updateMethod(
node,
node.decorators,
Expand All @@ -385,6 +391,8 @@ class Transformer {
) as any;
} else if (ts.isGetAccessorDeclaration(node) && node.body != null) {
const statements = this.getStatementsForDeclaration(node);
this.warningCallsWereInjected =
this.warningCallsWereInjected || statements.length > 0;
return ts.updateGetAccessor(
node,
node.decorators,
Expand All @@ -399,6 +407,8 @@ class Transformer {
) as any;
} else if (ts.isSetAccessorDeclaration(node) && node.body != null) {
const statements = this.getStatementsForDeclaration(node);
this.warningCallsWereInjected =
this.warningCallsWereInjected || statements.length > 0;
return ts.updateSetAccessor(
node,
node.decorators,
Expand All @@ -412,6 +422,8 @@ class Transformer {
) as any;
} else if (ts.isConstructorDeclaration(node) && node.body != null) {
const statements = this.getStatementsForDeclaration(node);
this.warningCallsWereInjected =
this.warningCallsWereInjected || statements.length > 0;
return ts.updateConstructor(
node,
node.decorators,
Expand Down
20 changes: 20 additions & 0 deletions packages/jsii/test/deprecation-warnings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,26 @@ describe('Call injections', () => {
);
});

test('does not generate a require statement when no calls were injected', async () => {
const result = await compileJsiiForTest(
{
'index.ts': `export * from './some/folder/handler'`,
'some/folder/handler.ts': `
export function handler(event: any) { return event; }
`,
},
undefined /* callback */,
{ addDeprecationWarnings: true },
);

const expectedPath = ['..', '..', '.warnings.jsii.js'].join('/');

const content = jsFile(result, 'some/folder/handler');
expect(content).not.toContain(
`const jsiiDeprecationWarnings = require("${expectedPath}")`,
);
});

test('deprecated methods', async () => {
const result = await compileJsiiForTest(
`
Expand Down

0 comments on commit 8f53f89

Please sign in to comment.