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

Add schematics to @ng-bootstrap/ng-bootstrap #3669

Merged
merged 1 commit into from
Apr 16, 2020

Conversation

maxokorokov
Copy link
Member

@maxokorokov maxokorokov commented Apr 7, 2020

Based on the latest version of the @ng-bootstrap/schematics (it will be deprecated)

Mostly technical PR, no new features/fixes:

  • copied code from @ng-bootstrap/schematics
  • made it work with Angular 9
  • made it work with our CI
  • updated generated @ng-bootstrap/ng-bootstrap package with built schematics

Fixes #2361

@benouat

@maxokorokov maxokorokov added this to the 6.1 milestone Apr 7, 2020
@maxokorokov maxokorokov changed the title schematics: initial version integrated into CI Add schematics to @ng-bootstrap/ng-bootstrap Apr 7, 2020
import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils';
import {getProject} from '@schematics/angular/utility/project';
import * as path from 'path';
import * as ts from 'typescript';
Copy link
Contributor

Choose a reason for hiding this comment

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

Recently fixed issue in another library, which was caused by the similar code. @schematics/angular bundles its own version of TS and as a result AST is created by the workspace version of TS (ts.createSourceFile() call), while it is modified by the version bundled by @schematics/angular. If these versions are different it crashes. The reproduction was to ng new my-proj with latest CLI, change workspace TS to 3.7.5 and ng add <lib>.

See FortAwesome/angular-fontawesome#238 for more details.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you, will check this

@maxokorokov maxokorokov reopened this Apr 10, 2020
@codecov-io
Copy link

Codecov Report

Merging #3669 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3669      +/-   ##
==========================================
- Coverage   91.53%   91.53%   -0.01%     
==========================================
  Files         100      100              
  Lines        2930     2928       -2     
  Branches      539      539              
==========================================
- Hits         2682     2680       -2     
  Misses        186      186              
  Partials       62       62              
Flag Coverage Δ
#e2e 56.86% <ø> (-0.03%) ⬇️
#unit 88.38% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
src/dropdown/dropdown.ts 94.07% <ø> (-0.04%) ⬇️
src/util/positioning.ts 95.86% <ø> (-0.04%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1640eb2...a0851fa. Read the comment docs.

@maxokorokov
Copy link
Member Author

maxokorokov commented Apr 15, 2020

Pushed the following changes:

  • support for @angular/localize polyfill installation
  • support for both .sass and .scss
  • more unit tests

Full list of what is supported at the moment:

  • ng add @ng-bootstrap/ng-bootstrap → will target a default CLI project
  • ng add @ng-bootstrap/ng-bootstrap --project xxx → will target project xxx
  • installing bootstrap, @ng-bootstrap/ng-bootstrap and @angular/localize dependencies
  • adding @angular/localize polyfill
  • adding NgbModule to the main application module
  • adding styles for .scss and .sass in the style file directly
  • warning for unsupported project styles for less, stylus
  • adding bootstrap.min.css to angular.json in case of CSS styles / unsupported project styles

What is still missing:

@benouat, could you please take a look at this?

Copy link
Member

@benouat benouat left a comment

Choose a reason for hiding this comment

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

I mostly agree with the code.
I left a few comments, essentially nitpicking ones ! 😜

misc/copy-static-files.ts Outdated Show resolved Hide resolved
schematics/collection.json Outdated Show resolved Hide resolved
schematics/ng-add/index.ts Outdated Show resolved Hide resolved
schematics/ng-add/schema.ts Outdated Show resolved Hide resolved
schematics/ng-add/steps/add-bootstrap.ts Show resolved Hide resolved
Based on the '@ng-bootstrap/schematics' latest version
@maxokorokov maxokorokov changed the base branch from master to schematics April 16, 2020 15:27
@maxokorokov maxokorokov merged commit 61ffa4c into ng-bootstrap:schematics Apr 16, 2020
maxokorokov added a commit that referenced this pull request Apr 17, 2020
- `ng add @ng-bootstrap/ng-bootstrap` with optional `--project`
- installing `bootstrap`, `@ng-bootstrap/ng-bootstrap` and `@angular/localize` dependencies
- adding `@angular/localize` polyfill
- adding `NgbModule` to the main application module
- adding styles for `.scss` and `.sass` in the style file directly
- warning for unsupported project styles for `less`, `stylus`
- adding `bootstrap.min.css` to `angular.json` in case of CSS styles / unsupported project styles
maxokorokov added a commit that referenced this pull request Apr 20, 2020
- `ng add @ng-bootstrap/ng-bootstrap` with optional `--project`
- installing `bootstrap`, `@ng-bootstrap/ng-bootstrap` and `@angular/localize` dependencies
- adding `@angular/localize` polyfill
- adding `NgbModule` to the main application module
- adding styles for `.scss` and `.sass` in the style file directly
- warning for unsupported project styles for `less`, `stylus`
- adding `bootstrap.min.css` to `angular.json` in case of CSS styles / unsupported project styles

fix(schematics): linting
@maxokorokov maxokorokov mentioned this pull request Apr 20, 2020
maxokorokov added a commit that referenced this pull request Apr 20, 2020
- `ng add @ng-bootstrap/ng-bootstrap` with optional `--project`
- installing `bootstrap`, `@ng-bootstrap/ng-bootstrap` and `@angular/localize` dependencies
- adding `@angular/localize` polyfill
- adding `NgbModule` to the main application module
- adding styles for `.scss` and `.sass` in the style file directly
- warning for unsupported project styles for `less`, `stylus`
- adding `bootstrap.min.css` to `angular.json` in case of CSS styles / unsupported project styles

fix(schematics): linting
@maxokorokov maxokorokov deleted the schematics branch January 14, 2021 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ng add with @ng-bootstrap/ng-bootstrap
4 participants