valor-software / ngx-bootstrap

Fast and reliable Bootstrap widgets in Angular (supports Ivy engine)
https://valor-software.com/ngx-bootstrap
MIT License
5.53k stars 1.69k forks source link

chore(ide): Constructor Parameter Injectables not working in Visual Studio #78

Closed UWouldlLose closed 8 years ago

UWouldlLose commented 8 years ago

Locally I haven't been able to get the ng2- components to work without updating the constructors to include @Inject() annotations and re-building.

Do you know if I'm doing something wrong?

Would it be ok to submit a pull request adding Inject() to the constructor parameters?

valorkin commented 8 years ago

Are you emitting metadata?

UWouldlLose commented 8 years ago

I should be emitting metadata. Using visual studio for development, systemjs-builder for packaging to deploy.

Here's my systemjs config used in visual studio:

                    baseURL: "/",
                    packages: {
                        'app': { defaultExtension: 'js' },
                        'src': { defaultExtension: 'js' },
                        'rxjs': { defaultExtension: 'js' },
                        'angular2': { defaultExtension: 'js' },
                        'ng2-charts': { defaultExtension: 'js' },
                        'ng2-bootstrap': { defaultExtension: 'js' }
                    },
                    paths: {
                        'rxjs/observable/*': '/node_modules/rxjs/add/observable/*',
                        'rxjs/operator/*': '/node_modules/rxjs/add/operator/*',
                        'rxjs/*': '/node_modules/rxjs/*',

                        'angular2/*': '/node_modules/angular2/*',

                        'ng2-charts/*': '/node_modules/ng2-charts/*',
                        'ng2-charts': '/node_modules/ng2-charts/ng2-charts.js',

                        'ng2-bootstrap/*': '/node_modules/ng2-bootstrap/*',
                        'ng2-bootstrap': '/node_modules/ng2-bootstrap/ng2-bootstrap.js'
                    }
                });

So if I reference the .js files off npm directly I see errors about the parameters for type xyz aren't able to be resolved. Adding @Inject() annotations to the typescript files and re-building the .js gets things working.

valorkin commented 8 years ago

which version of typescript and angular2 are you using?

ludohenin commented 8 years ago

@UWouldlLose Typescript emits metadata only if it has to, for instance when using a decorator. The best practice for classes using Angular DI is to add the Injectable decorator to that class.

@Injectable() // Does nothing but force ts to emit metadata (also for constructor params)
export MyClass {
  constructor(public http: Http) {}
  //...
}
valorkin commented 8 years ago

@ludohenin I was not facing this problem for a long time with tsc >1.7 nor with webpack nor with system.js so I am not sure about need to add Inject everywhere

UWouldlLose commented 8 years ago

@valorkin tsc -version yields message TS6029: Version 1.7.3

I'm on angular 2.0.0-beta.0

Allot of the example code I see floating around online doesn't use Injectable either. Maybe it's related to our dev tools? Visual studio vs. xyz

valorkin commented 8 years ago

any luck make this working?

UWouldlLose commented 8 years ago

No progress on my side. I'll pull down the release version of ng2-bootstrap and see if everything works right on my end using the command line typescript compiler.

If it does, visual studio must be doing something weird.

valorkin commented 8 years ago

ok, keep me updated please :)

UWouldlLose commented 8 years ago

I did some testing yesterday, bouncing between beta0 and beta1. It looks like the current library versions won't run for me in both of my environments on beta0.

Local slideparameters

Dev devexception

Any suggestion on things I could try out, or other info I could provide that would help?

valorkin commented 8 years ago

tsconfig?

UWouldlLose commented 8 years ago

I'm not using a tsconfig at the moment :-). It may be a weird way to do things, but has been working well.

The attached zip contains a gulpfile I'm using to build the app and the two system.config's it's using.

system.config.vendorbuild.js is used in the compile:dependencies gulp task to bundle all the non-app javascripts into one file.

I'm pretty much just importing library javascript files, and only building my own app code from typescript.

And then in my index file, I import the bundles.

<!DOCTYPE html>
<html>
<head>
    @{
        string siteVersion = System.Web.Configuration.WebConfigurationManager.AppSettings["siteVersion"];
        string environment = System.Web.Configuration.WebConfigurationManager.AppSettings["environment"];
    }

    @{
        if (environment == "local")
        {
            <link href="/node_modules/bootstrap/dist/css/bootstrap.min.css?v=@siteVersion" rel="stylesheet" />
            <link href="/assets/css/site.css?v=@siteVersion" rel="stylesheet" />
        }
        else
        {
            <link href="/dist/vendor/css_bundle.css?v=@siteVersion" rel="stylesheet" />
        }
    }

    </head>
    <body>
        <my-app>loading...</my-app>

        <script src="/dist/vendor/deps_bundle.js?v=@siteVersion"></script>
        <script src="/dist/vendor.js?v=@siteVersion"></script>

        @{
            if (environment == "local")
            {

                <script>

                    System.config({
                        baseURL: "/",
                        packages: {
                            'app': { defaultExtension: 'js' },
                            'src': { defaultExtension: 'js' },
                            'rxjs': { defaultExtension: 'js' },
                            'angular2': { defaultExtension: 'js' },
                            'ng2-charts': { defaultExtension: 'js' },
                            'ng2-bootstrap': { defaultExtension: 'js' }
                        },
                        paths: {
                            'rxjs/observable/*': '/node_modules/rxjs/add/observable/*',
                            'rxjs/operator/*': '/node_modules/rxjs/add/operator/*',
                            'rxjs/*': '/node_modules/rxjs/*',

                            'angular2/*': '/node_modules/angular2/*',

                            'ng2-charts/*': '/node_modules/ng2-charts/*',
                            'ng2-charts': '/node_modules/ng2-charts/ng2-charts.js',

                            'ng2-bootstrap/*': '/node_modules/ng2-bootstrap/*',
                            'ng2-bootstrap': '/node_modules/ng2-bootstrap/ng2-bootstrap.js'
                        }
                    });

                    System.import('/src/app.js');

                </script>
            }
            else
            {
                <script src="/dist/bundledapp.js?v=@siteVersion"></script>

                <script>
                    System.import('src/app.ts').catch(function (err) {
                        console.error(err);
                    });
                </script>
            }
        }

    </body>

</html>

gulpfile.zip

UWouldlLose commented 8 years ago

One other note if it's relevant.

If I tweak the build to use the .ts files instead of .js things work as expected with systemjs-builder.

System.config({
    transpiler: 'typescript',
    typescriptOptions: {
        emitDecoratorMetadata: true
    },
    map: {
        app: 'src',
        typescript: 'node_modules/typescript/lib/typescript.js',
        angular2: 'node_modules/angular2',
        rxjs: 'node_modules/rxjs'
    },
    paths: {
        'ng2-charts/*': '/node_modules/ng2-charts/*',
        'ng2-charts': '/node_modules/ng2-charts/ng2-charts.ts',

        'ng2-bootstrap/*': '/node_modules/ng2-bootstrap/*',
        'ng2-bootstrap': '/node_modules/ng2-bootstrap/ng2-bootstrap.ts'
    },
    packages: {
        app: {
            defaultExtension: 'ts',
            main: 'app.ts'
        },
        angular2: {
            defaultExtension: 'js'
        },
        rxjs: {
            defaultExtension: 'js'
        },
        'ng2-charts': {
            defaultExtension: 'ts'
        },
        'ng2-bootstrap': {
            defaultExtension: 'ts'
        }
    }
});
UWouldlLose commented 8 years ago

Bah, or not, after re-booting everything I'm back to modifying the .ts files.

Just wanted to keep this updated.

valorkin commented 8 years ago

I will be writing a doc about, may be it could be helpful

UWouldlLose commented 8 years ago

I pulled down the latest 1.0.2-beta.1 code to get the fix mentioned here . My trick with updating the .ts files to include @Inject statements doesn't seem to be working with slides split out as a separate component. Comparing the modified vs. original file's compiled output yields zero differences which seems weird.

It's not a huge problem for me, but it would be nice to figure out what's going on. Anything I could do to debug/troubleshoot let me know.

valorkin commented 8 years ago

@UWouldlLose please check this https://github.com/valor-software/angular2-quickstart is it working or not? :)

UWouldlLose commented 8 years ago

@valorkin Thanks for the link!

I think you're right about not needing @Inject in the library code.

Comparing your sample project with mine, the main difference I saw was loading ng2-bootstrap.min.js through a script tag vs. systemjs.

I've changed my local config around to load the bundled bootstrap library and the carousel is running smoothly.

No errors yay :-)

valorkin commented 8 years ago

Horrey! I am closing this :)