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

feat: support signal input from angular 17.1 #8818

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

andreandersson
Copy link

Adds a custom transformer to transform Angular Code, specifically, signals (input, query and models) to behave better in jit-environment.

Solves #7976

@andreandersson
Copy link
Author

@satanTime the CI will probably not work "as is", and I'm not sure how to solve this nicely. We need to run a "pre-build-step" to downgrade angularJitApplicationTransform to cjs instead of esm. I'm guessing we need to update all test / build-steps in the scripts of package.json?

@andreandersson andreandersson force-pushed the bugfix/7976 branch 2 times, most recently from c40996f to 866af7e Compare April 25, 2024 06:58
Copy link

codecov bot commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (940dc1a) to head (e519230).

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #8818   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          227       227           
  Lines         4935      4925   -10     
  Branches      1147      1142    -5     
=========================================
- Hits          4935      4925   -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andreandersson andreandersson force-pushed the bugfix/7976 branch 11 times, most recently from 2c78953 to d49e2a0 Compare May 20, 2024 12:02
André Andersson and others added 2 commits May 20, 2024 14:05
Adds a custom transformer to transform Angular Code, specifically,
signals (input, query and models) to behave better in jit-environment.

Solves help-me-mom#7976

Signed-off-by: André Andersson <[email protected]>
Removes some dead code for collecting parameters. Angular jit application transform
downlevels constructors and associated Angular decorators.

Signed-off-by: André Andersson <[email protected]>
This is handled by Angulars JIT Application Transform.

Signed-off-by: André Andersson <[email protected]>
@andreandersson
Copy link
Author

This is finally ready for review

@satanTime
Copy link
Member

Thank you, I'll take a look during the next days.

@satanTime
Copy link
Member

Could you explain the changes when you have time?

@andreandersson
Copy link
Author

andreandersson commented May 21, 2024

Sure, though I'm not really sure I understand the changes myself to a full extend I'll do my best.

The most important part is running angularJitApplicationTransform when transpiling ng-mocks. angularJitApplicationTransform is part of @angular/core and downlevels code by, for example, changing from a signal input to @Input.

Since @angular/core is an esm-module we need to convert it to CommonJS to be able to build it for ng-mocks, hence why we're not simply importing it from @angular/core directly but rather use esbuild to convert it in a post-install step. I'm very open to other suggestions here, since this is probably the thing I dislike most about the current solution.

Since the transforming does other things I was able to remove some more code. Mainly parseParameters which handles @Attribute. These are some changes I'm not to confident about, but the tests seem to think the solution is fine.

The usage of angularJitApplicationTransform is needed twice, one for the tests (karma.conf.ts) and one for "real world" (webpack.config.js). In other words; we don't really have any tests for the actual implementation. Only that the test-configuration is correct. The test is located in e2e a18, though it should run in all 17.1+ versions of angular. The only reason for this was problems with import { input } from '@angular/core' in different versions of angular while still having the angularJitApplicationTransform handling the typings in a good way.

The solution is quite inspired from jest-preset-angular: thymikee/jest-preset-angular#2303

Let me know if you want to talk to me somewhere else regarding these changes. I'll try to update the commit messages with this information as well.

@SiimTa
Copy link

SiimTa commented Aug 5, 2024

@satanTime Are there any updates about this pull request? The amount of NG0303 errors generated in specs by signal inputs can cause a lot of headache :( Related issue: #7976 (comment)

@Jessy-BH
Copy link

How tf this PR isn't merged yet ? We kinda need this to use your package with angular 17-18

@thiboot
Copy link

thiboot commented Oct 30, 2024

Hello, seems we need this in my team projects. Any news ?

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.

5 participants