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

fix: improve instanceof checks to support custom transformers #9970

Closed
wants to merge 1 commit into from
Closed

fix: improve instanceof checks to support custom transformers #9970

wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 11, 2022

This commit updates two instanceof checks to instead check
the constructor name. This is done to better support custom
transformers. Because custom transformers import/require
potentially different copies of the transformer modules,
the instanceof checks do not work. This required v2 custom
transformers to rely on NODE_PATH hacks and other
workarounds.

Fixes: #9362

@cjihrig cjihrig requested a review from lazpavel March 11, 2022 22:15
@cjihrig cjihrig requested a review from a team as a code owner March 11, 2022 22:15
@codecov-commenter
Copy link

Codecov Report

Merging #9970 (8a76661) into master (3df3af0) will increase coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #9970      +/-   ##
==========================================
+ Coverage   54.17%   54.21%   +0.04%     
==========================================
  Files         835      834       -1     
  Lines       46117    46068      -49     
  Branches     9839     9835       -4     
==========================================
- Hits        24982    24975       -7     
+ Misses      19144    19105      -39     
+ Partials     1991     1988       -3     
Impacted Files Coverage Δ
...phql-transformer-core/src/cdk-compat/file-asset.ts 16.00% <0.00%> (-3.24%) ⬇️
...ansformer-core/src/cdk-compat/stack-synthesizer.ts

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@lazpavel lazpavel left a comment

Choose a reason for hiding this comment

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

LGTM

This commit updates two instanceof checks to instead check
the constructor name. This is done to better support custom
transformers. Because custom transformers import/require
potentially different copies of the transformer modules,
the instanceof checks do not work. This required v2 custom
transformers to rely on NODE_PATH hacks and other
workarounds.

Refs: #9362
@cjihrig cjihrig reopened this Mar 29, 2022
Copy link
Contributor

@sundersc sundersc left a comment

Choose a reason for hiding this comment

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

LGTM.

@johnpc
Copy link
Contributor

johnpc commented Apr 11, 2022

closing in favor of #10188 as this change cannot be rebased due to @cjihrig's fork of amplify-cli no longer existing.

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.

Custom Transformer v2 - Error: Template asset can be used only with TransformerStackSynthesizer
7 participants