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

refactor(angular): refactored angular related imports generation #751

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

g-cheishvili
Copy link
Contributor

Description

  • added populatedWithAngularImports and moved adding Angular related imports to be BEFORE json.post call.
  • added one helper for adding an import to MitosisComponent. I do not know if something like it existed, could not find it, so added it. if there is any, please refer me to it.

Reasoning

I added these because otherwise in json.post you can not see which imports are gonna end up in file completely. Also, output code's structure is now better because it follows a widely accepted pattern:

  • imports
  • types
  • anything else
  • class declaration

@g-cheishvili g-cheishvili requested review from a team and samijaber as code owners September 23, 2022 07:30
@vercel
Copy link

vercel bot commented Sep 23, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
mitosis-fiddle ✅ Ready (Inspect) Visit Preview Sep 23, 2022 at 8:47PM (UTC)

* @param json The component to add the import to
* @param theImport The import to add
*/
export function addImportToMitosisComponent(
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have this yet. Thanks for putting it together!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aand I need to update snapshots here as well

@g-cheishvili
Copy link
Contributor Author

@samijaber fixed everything. Take a look

@samijaber
Copy link
Contributor

@g-cheishvili I believe this is still pending updating snapshots?

@PatrickJS
Copy link
Collaborator

@g-cheishvili any updates on this?

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.

None yet

3 participants