ngx-admin-lte icon indicating copy to clipboard operation
ngx-admin-lte copied to clipboard

Explicit dependency on jquery is not necessary, should be removed IMHO

Open catull opened this issue 7 years ago • 16 comments

In 2.0.0-beta.9 jquery was removed as a direct dependency, it is an indirect dependency.

In 2.0.0-beta.10 jquery was re-introduced.

Fact is, you don't need to specify this dependency.

Because if you leave it out, admin-lte has a dependency on jqeury and will pick it up. You end up having node_modules/jquery anyway.

For the same reason, we should NOT explicitly include bootstrap.

Why not ?

Because jquery and bootstrap are transitive dependencies of our dependencies, most likely admin-lte.

[email protected] depends on a specific bootstrap and jquery version. [email protected] depends on slightly different bootstrap and jquery version.

All I am saying is, let [email protected] figure out which sub-dependencies it has. If we specify a certain version, we are only asking for more maintenance on OUR side.

Where is the benefit of that ?

My web app has never specified jquery EXPLICITLY, because admin-lte and/or bootstrap has always had that responsibility.

Guess what, bootstrap 4 apparently has no more direct dependency on jquery.

It has a peerDependency relationship though, see: bootstrap dependency on jquery

catull avatar Mar 12 '18 20:03 catull

beta.9 was not installing jquery at all

Resulting in this error during the "yarn prod" process: An error occured during the build: Error: ENOENT: no such file or directory, open '/Users/TwanoO/Documents/PERSO/GIT/bootstraping-ngx-admin-lte/node_modules/jquery/dist/jquery.js'

That's why it's re-introduced in beta.10

TwanoO67 avatar Mar 12 '18 20:03 TwanoO67

That's because we still are on admin-lte 2.3 resulting in that dependencies (where no bootstrap nor jquery are): https://github.com/almasaeed2010/AdminLTE/blob/v2.3.11/package.json

As soon as we fixed the issue #25, we could only depend on admin-lte 2.4 (and its own subdependencies for jquery and bootstrap)

TwanoO67 avatar Mar 12 '18 20:03 TwanoO67

I see, so the goal is the same, just not achievable with admin-lte 2.3.x.

catull avatar Mar 12 '18 21:03 catull

About transitive dependency matter, can I ask you a question?

I'm trying to build a coreProject that imports ngx-admin-lte and add other some services and components of mine that I should use in all my projects. I build coreProject and export it.

In all my projects that include coreProject (with embedded ngx-admin-lte) I would like:

  • to customize header and footer with logoService and footerService
  • to define menu with userMenu
  • to define routes using LayoutAuthComponent
  • to inherit all utilities of coreProject that interact with theme on init.

But... I got error building the projects, because they have not access to components and services of ngx-admin-lte. Is there a way to have ngx-admin-lte passing through my coreProject ?

fabioformosa avatar Mar 15 '18 00:03 fabioformosa

@fabioformosa Excellent points you bring up.

Just the last two days I had to put in a hack, just to override the HTML template of the user-box.

I wished there was a way to specify either a different Component or at least just provide a different layout, similar to your first bullet point.

catull avatar Mar 15 '18 03:03 catull

@fabioformosa

On the menu and routes, I was able to define my own menu, along with routes to my own components.

Can you be more specific about the errors, I am afraid I cannot follow your concern.

Show us the error, please.

Does project A which inherits from corePropject not see service "Admin LTE User service" for instance ?

Is that a case ?

Because there is a solution for that, it is called re-exporting.

catull avatar Mar 15 '18 03:03 catull

@fabioformosa

Ideally, you should not have to export components from ngx-admin-lte out of your coreProject in the first place.

Rather, have services from coreProject offer your own components and services only; your own components and services conceal the "inner" components and services.

catull avatar Mar 15 '18 03:03 catull

Hi @catull , I'd start by saying that I'm rather a newbie in angular, I've just begun to convert a coreProject from angularJS to angular.

Answering to your request of further details, I have: projectA -> coreProj -> ngx-admin-lte

I would like to carry-out this scenario:

coreProj structure:
 |
 |- coreThemeModule -> ngx-admin-lte
 |- coreUtilityModule -> services, components (they could interact with theme)

ProjectA should use theme offered by coreThemeModule "as-is", except for menu and header/footer customization. CoreProject should:

  • place a router-outlet in "main content box" of ngx-admin-lte main layout, to allow ProjectA to define routes only for inner content
  • manage all common tasks: user profile section, user box for logged user, login/signup redirect, ecc.

ProjectA shouldn't have ngx-admin-lte in package.json, neither admin-lte, bootstrap. I think that should be transitive dependencies. However ProjectA needs to access to menuService, footerService, logoService offered by ngx-admin-lte, only importing (passing-through) CoreProject.

At the moment the error I got compiling ProjectA is: CoreProject has no exported member 'FooterService'.

Should coreProject export ngx-admin-lte modules? Should CoreProject wrap ngx-admin-lte modules?

(I'm interested at your hack to customize user profile box, it will be my next task as well. Maybe it's worth to open an other issue to address this one)

fabioformosa avatar Mar 15 '18 09:03 fabioformosa

ProjectA indeed won't have to know about ngx-admin-lte and all its dependencies.

My setup is a bit simpler, I have myApp -> ngx-admin-lte. For that to work, I have some "hacks" in myApp's .angular-cli.json:

        "styles": [
            "styles.css",
            "assets/integrator-theme.css",
            "../node_modules/bootstrap/dist/css/bootstrap.css",
            "../node_modules/ionicons/dist/css/ionicons.css",
            "../node_modules/admin-lte/dist/css/AdminLTE.css",
            "../node_modules/admin-lte/dist/css/skins/_all-skins.css",
            "../node_modules/@swimlane/ngx-datatable/release/index.css",
            "../node_modules/@swimlane/ngx-datatable/release/themes/material.css",
            "../node_modules/@swimlane/ngx-datatable/release/assets/icons.css",
            "../node_modules/font-awesome/css/font-awesome.css"
        ],
        "scripts": [
            "../node_modules/jquery/dist/jquery.js",
            "../node_modules/admin-lte/plugins/jQueryUI/jquery-ui.js",
            "../node_modules/bootstrap/dist/js/bootstrap.js",
            "../node_modules/ace-builds/src/ace.js",
            "../node_modules/ace-builds/src/ext-language_tools.js",
            "../node_modules/ace-builds/src/ext-split.js",
            "../node_modules/ace-builds/src/mode-java.js",
            "../node_modules/ace-builds/src/mode-json.js",
            "../node_modules/ace-builds/src/mode-xml.js",
            "../node_modules/ace-builds/src/theme-idle_fingers.js",
            "../node_modules/ace-builds/src/theme-github.js",
            "../node_modules/ace-builds/src/theme-xcode.js",
            "../node_modules/admin-lte/dist/js/app.js",
            "assets/js/scripts.js"
        ],

catull avatar Mar 15 '18 09:03 catull

What does coreProject's main module provider: section look like ?

catull avatar Mar 15 '18 09:03 catull

ok. ProjectA -> CoreProject -> ngx-admin-lte

ProjectA needs to use LogoService of ngx-admin-lte but currently ProjectA can't use it. At the moment, only CoreProject can use LogoService.

I tried

import {
    User,
    MenuService,
    LogoService,
    FooterService
} from 'ngx-admin-lte';

@NgModule({
  imports: [
    CommonModule,
    RouterModule,
    NgxAdminLteModule
  ],
  declarations: [coreProjectComponent],
  exports: [
    coreProjectComponent,
    User,
    MenuService,
    LogoService,
    FooterService
  ]
})
export class CoreProjectModule { }

but it seems it's not the way

fabioformosa avatar Mar 15 '18 11:03 fabioformosa

import {
    User,
    MenuService,
    LogoService,
    FooterService
} from 'ngx-admin-lte';

@NgModule({
  imports: [
    CommonModule,
    RouterModule,
    NgxAdminLteModule
  ],
  declarations: [coreProjectComponent],
  exports: [
    coreProjectComponent,
    User
  ],
  providers: [
    MenuService,
    LogoService,
    FooterService
  ]
})
export class CoreProjectModule { }

Any better ?

catull avatar Mar 15 '18 13:03 catull

Services and guards are provided, components are exported.

catull avatar Mar 15 '18 13:03 catull

Oops a little oversight. However I don't get the target yet. I created 2 repos:

  • kaishi https://github.com/fabioformosa/kaishi
  • simple-kaishi https://github.com/fabioformosa/simple-kaishi

simple-kaishi -> kaishi (coreProject) -> ngx-admin-lte

I've opened an issue to avoid OT at this issue https://github.com/fabioformosa/simple-kaishi/issues/1

Any help is appreciated.

I've not published kaishi in npm registry, so you should:

  • launch on kaishi npm packagr
  • create a local link with npm link inside simple-kaishi against your path to 'kaishi/dist'

fabioformosa avatar Mar 15 '18 15:03 fabioformosa

Update: Kaishi is now a core library that wraps ngx-admin-lte and it could export other core service as well. Feedback: Projects, that use kaishi as core library, need to import ngx-admin-lte only to access to css and js (jquery, bootstrap). The idea, at the basis of Kaishi, was instead to hide theme implementation.

Getting back to the subject of this issue, as far as you know, is there a way thorugh ngx-admin-lte can include resources (styles, and others) in its bundle?

fabioformosa avatar Mar 16 '18 10:03 fabioformosa

As we're now providing APF format, we could maybe do something with the main "style.css", and probably manage to import that.

In the same way, we can probably import jquery,bootstrap js etc... in a main js file, for this import to be hidden.

TwanoO67 avatar Mar 19 '18 08:03 TwanoO67