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: CLI code generation in ng11.1+ NS-only projects #314

Merged
merged 3 commits into from
Apr 21, 2021

Conversation

agonper
Copy link
Contributor

@agonper agonper commented Apr 21, 2021

PR Checklist

What is the current behavior?

  1. @schematics/angular v11.1+ has introduced a breaking change where @schematics/angular/utility/config has been removed. @angular/cli v11.1+ seems automatically install these new versions. In v11.1+ version of @schematics/angular, getWorkespace method is instead at @schematics/angular/utility/workespace and is async now as pointed out at Can't use Angular CLI with fresh nativescript installation #311 by @avif. Using it requires a major codebase migration. Forcing this version (~11.0.0) via regular dependencies allows to continue using @nativescript/schematics with NG11.1 and NG11.2 projects, but further action should be taken before NG12 arrives.
  2. There was a bug in the code generation utils by which every NS NG project counted as a shared project, thus generating web components in NS-only projects. The isWeb() utility function was considering the existence of a nativescript.config.ts file as a sign of a shared project configuration, when now every NS project has this file. Instead now this decision is driven by the existence of a main.tns.ts file inside the src folder.
  3. There was a regression which lead to including the .tns extension in all NG NS components during their generation, independently of the kind of project (shared or NS only). This extension should be used in NS component files inside shared projects only.
  4. Given all NS NG templates use sass preprocessor with scss extension, it should be stated in the docs how to configure angular.json file to make this the default style sheets extension during component generation.

What is the new behavior?

  1. @schematics/angular dev dependency has been temporally promoted to become a regular dependency thus forcing a specific version range which still works with @nativescript/schematics in NG11.1+ projects as expected with no extra setup steps (like manually adding this dependency to the package.json of the project)
  2. The isWeb()utility function has been refactored to detect the existence of a main.tns.ts file instead.
  3. When run inside a shared project getExtensions() utility function returns different extensions for NS and web components, otherwise it returns blank extension prefixes for NS-only projects.
  4. Docs have been updated to reflect this possibility.

Fixes/Implements/Closes #311, #309 and #307.

BREAKING CHANGES:
None, to the best of my knowledge.

Migration steps:
None, since by merging this changes it will be possible to use @nativescript/schematics with NG11.1+ projects as expected by just following setup instructions (with no extra steps or workaround like manually installing @schematics/angular@~11.0.0).


This is my first PR, I don't know if something else is needed (please, reach me out if so). I just wanted to help making this working back again for new projects

agonper added 3 commits April 21, 2021 12:43
…lar dependencies

@schematics/angular v11.1+ induced a regression where @schematics/angular/utility/config has been removed. getWorkespace method is instead at @schematics/angular/utility/workespace and is async now. Using it requires a major codebase migration. Forcing this version (~11.0.0) via regular dependencies allows to continue using @nativescript/schematics with NG11.1 and NG11.2 projects
… projects

All NS NG templates come with SCSS configured by default. Adding these lines to angular.json file avoid having to specify --style=scss option every time during component creation.
@NathanWalker
Copy link
Contributor

This is amazing to see @agonper - we will bump a version update out here today with this.

@NathanWalker NathanWalker merged commit ab4f9f2 into NativeScript:master Apr 21, 2021
@agonper agonper deleted the fix-ng11.1+ branch April 21, 2021 16:30
@NathanWalker
Copy link
Contributor

@agonper this is published with 11.2.0-rc.0 ('rc' tag) - if you could try it and if all works for you we can make final. Really appreciate your help. It was built with latest 11.2.0 and can see the additional changes that were needed here:
3ff5fa7
due to getWorkspace becoming async.

@agonper
Copy link
Contributor Author

agonper commented Apr 22, 2021

Edit: TL;DR

See next comment


Hi @NathanWalker, thanks for giving priority to this. I've just tested the new version and sadly it doesn't work.

I'm seeing the following error when trying to generate components and modules:
Cannot read property 'kind' of undefined

Findings

I know that error is not very informative. So I've entered the rabbit hole and these are my findings:
The error is thrown by two utility functions available at @schematics/angular/utility/ast-utils, so this seems to be yet another breaking change in @schematics/angular.

In the case of the module generation it happens while calling the function addSymbolToNgModuleMetadata here.
image

In the case of the component generation it happens while calling the function addDeclarationToModule here.
image

The later addDeclarationToModule ends up using the first one addSymbolToNgModuleMetadata (see here).
image

So I've checked the addSymbolToNgModuleMetadata (code), to see where the problem could be. This function uses the getDecoratorMetadata function. Which in turn uses the findNodes and _angularImportsFromNode functions. See screenshots:
image
image

And finally the error is thrown here, in the _angularImportsFromNode function:
image

It seems that the node is missing the moduleSpecifier property, hence ms is undefined:

NodeObject {
  pos: 6,
  end: 19,
  flags: 0,
  modifierFlagsCache: 0,
  transformFlags: 0,
  parent: NodeObject {
    pos: 6,
    end: 19,
    flags: 0,
    modifierFlagsCache: 0,
    transformFlags: 0,
    parent: NodeObject {
      pos: 0,
      end: 41,
      flags: 0,
      modifierFlagsCache: 0,
      transformFlags: 0,
      parent: [SourceFileObject],
      kind: 258,
      decorators: undefined,
      modifiers: undefined,
      symbol: undefined,
      localSymbol: undefined,
      locals: undefined,
      nextContainer: undefined,
      importClause: [Circular],
      moduleSpecifier: [TokenObject], <--- First appearance 
      _children: [Array]
    },
    kind: 259,
    isTypeOnly: false,
    name: undefined,
    namedBindings: [Circular],
    _children: [ [Circular] ]
  },
  kind: 261,
  elements: [
    NodeObject {
      pos: 8,
      end: 17,
      flags: 0,
      modifierFlagsCache: 0,
      transformFlags: 0,
      parent: [Circular],
      kind: 262,
      propertyName: undefined,
      name: [IdentifierObject]
    },
    pos: 8,
    end: 17,
    hasTrailingComma: false,
    transformFlags: 0
  ]
}

However its grand parent has it ¯_(ツ)_/¯

Guesses

It seems to be a TypeScript problem. I've checked in the @schematics/angular repo and they're using TypeScript 4.2. So I've bumped the TS version of my project to that version and it has temporally worked, but then I could not compile the project because Angular 11.2 requires a TS version lower than 4.2. However, I've tried to replicate that moment where it was temporally working and I haven't been able, there must be something else.

Hope all this information helps.

Proposal

  • Release the changes I contributed yesterday as v11.0.1 given it works with Angular 11.2 too (even when the dependencies were 11.0.0). There seems to be some interest from the community in having this working as soon as posible (3 issues, with multiple thumbs up)
  • Try to work on the migration to @schematics/angular v11.2 bit by bit, since there's no real pressure, given with the previous changes it already worked with Angular 11.2 and Angular 12 has not been yet released.
  • fix: lookup default stylesheet extension from angular.json file #315 & 🚀🚀!!

@agonper
Copy link
Contributor Author

agonper commented Apr 22, 2021

Ok, so I've found why I couldn't reproduce the moment when it worked. It seems it was I had the schematics project linked to my project locally to be able to debug the code a bit better and that was generating conflicts with the different TypeScript versions.

If I install @nativescript/[email protected] and I bump my TypeScript project version from ~4.0.0 to ~4.1.0, it works! 🎉

The only issue I've found is that default style sheet extension configuration seems to be broken now, but I found why. I'm going to try to fix it and I'll open a PR with the changes.

So, in summary, people with NG11 projects will have to use TypeScript 4.1 instead of 4.0

@agonper
Copy link
Contributor Author

agonper commented Apr 22, 2021

Everything should be fixed now at #315 😃

@NathanWalker
Copy link
Contributor

11.2.0 published now. Thanks again, let us know if see anything else in the final.

@agonper
Copy link
Contributor Author

agonper commented Apr 23, 2021

Works like a charm. Many thanks for the release @NathanWalker!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't use Angular CLI with fresh nativescript installation
2 participants