vinaygopinath / ng2-meta

Dynamic meta tags and SEO in Angular2
MIT License
201 stars 42 forks source link

Upgrade to work on ng2 Final. #11

Closed bencameron00 closed 8 years ago

bencameron00 commented 8 years ago

Will this component be upgraded to work on 2.0 Final? It was great on RC4 but hasn't worked since.

vinaygopinath commented 8 years ago

Hi, sorry for the delay! I'm aware that ng2-meta has issues on RC5 and beyond. I haven't had the time to keep up with the pace of changes in the ng2 ecosystem, but I'm getting around to it - I've started working on updating ng2-meta to support angular 2.0.0, and I hope to publish a new version in the middle of next week.

Thanks for using ng2-meta

bencameron00 commented 8 years ago

Cool, we’re waiting for it, it great, hope you get it out soon!

Thanks

Ben

From: Vinay Gopinath [mailto:notifications@github.com] Sent: 22 September 2016 17:26 To: vinaygopinath/ng2-meta ng2-meta@noreply.github.com Cc: Ben ben_cameron00@hotmail.com; Author author@noreply.github.com Subject: Re: [vinaygopinath/ng2-meta] Upgrade to work on ng2 Final. (#11)

Hi, sorry for the delay! I'm aware that ng2-meta has issues on RC5 and beyond. I haven't had the time to keep up with the pace of changes in the ng2 ecosystem, but I'm getting around to it - I've started working on updating ng2-meta to support angular 2.0.0, and I hope to publish a new version in the middle of next week.

Thanks for using ng2-meta

β€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vinaygopinath/ng2-meta/issues/11#issuecomment-248954918 , or mute the thread https://github.com/notifications/unsubscribe-auth/AFtLSdNWiJJTuDFKG4wlQW7fCR8sSTLlks5qsqwlgaJpZM4KEEt0 . https://github.com/notifications/beacon/AFtLScDbTpqZ9HY3P7wUEoZOVNFAunksks5qsqwlgaJpZM4KEEt0.gif

bjornharvold commented 8 years ago

+1 Good work here

bencameron00 commented 8 years ago

Hi, do you have a timeframe on when this is going to be complete? We are really looking forward to getting it into our latest build.

The site we are building is www.tilecase.com.

If you are interested in it we would love for you to follow us on LinkedIn at https://www.linkedin.com/company/4864276

sunshineo commented 8 years ago

The only problem I see is this

Property 'firstChild' does not exist on type 'RouterState'

I hope it is not too hard to fix.

vinaygopinath commented 8 years ago

It's not just the router/RouterState that's undergone changes - angular2 final itself has plenty of changes (from RC5). Also, I've tried to create MetaModule as a wrapper for MetaService, but it comes with its own complications, and I've run into some errors that I'm having trouble debugging. Would anyone be interested in taking a look? @sunshineo @bencameron00 @bergben

The angular2-final-compatible code for ng2-meta is available (as a draft) on the feature/upgrade-to-angular2-final branch. You can install that via npm

npm install --save vinaygopinath/ng2-meta#feature/upgrade-to-angular2-final

Here's how it would be used (in an app generated by angular-cli):

angular-cli.json

"scripts": [
  "../node_modules/ng2-meta/dist/index.js"
]

typings.d.ts

declare module 'ng2-meta';

app.module.ts

import { MetaModule } from 'ng2-meta';

@NgModule({
  declarations: [
    AppComponent
  ],
  imports: [
    BrowserModule,
    HttpModule,
    MetaModule.forRoot()
  ],
  providers: [],
  bootstrap: [AppComponent]
})
export class AppModule { }

and then you can inject MetaService into your main component, as usual.

Unfortunately, this throws the following error: Unexpected value 'undefined' imported by the module 'AppModule

Sorry that it's taken so long, and thanks for your help!

sunshineo commented 8 years ago

@vinaygopinath I don't know how to fix problems but sometimes I find ways to avoid them. I commented out the whole route change subscription part that contains the firstChild problem and switched to programmatically set the title and meta for all my pages and it worked fine for me.

Personally, I don't see the point of allowing them to write some json at the top of the component when it's the same to write couple JS lines in the ngOninit function right below.

bergben commented 8 years ago

Hey there @vinaygopinath, if I remember right I encountered the same error when I created https://github.com/bergben/ng2-scrollimate, so I am gonna have a look either today or tomorrow. Will report back here.

sunshineo commented 8 years ago

@vinaygopinath Have a look at https://github.com/angular/angular/issues/9662#issuecomment-252130872 which point to https://gist.github.com/alexbyk/0801373112b64bd171721c4b6b23e9e2

bergben commented 8 years ago

@vinaygopinath why are you using the forRoot here? because the MetaModule uses the RouterModule I suppose and you dont want to provide that again?

I don't know if I understood the concept of forRoot correctly...

Didn't have enough time yet to check for more but will do ASAP, sorry guys

vinaygopinath commented 8 years ago

The idea behind using forRoot is that MetaService is a singleton service that should be available throughtout the app as a single instance. It should not be possible to inject multiple instances of the service into different components.

Also, I'd like to pass the configuration of MetaService as a parameter of the forRoot method.

References: 1 2 3.

bergben commented 8 years ago

Hey guys! So I finally found the time to have a deeper look into this, sorry it took me so long. I've just created a pull reqeust #12 which should resolve this issue. I also implemented the option to pass in the metaConfig optionally.

This is how the app.module.ts would look like:

import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { FormsModule } from '@angular/forms';
import { HttpModule } from '@angular/http';
import { RouterModule } from '@angular/router';
import { MetaConfig, MetaModule } from 'ng2-meta';
import { AppComponent } from './app.component';
import { FeatureComponent } from './feature.component';

let metaConfig = new MetaConfig({
  //Append a title suffix such as a site name to all titles
  //Defaults to false
  useTitleSuffix: true,
  defaults: {
    title: 'Default title for pages without meta in their route',
    titleSuffix: ' | Site Name',
    'og:image': 'http://example.com/default-image.png',
    'any other': 'arbitrary tag can be used'
  }
});

@NgModule({
  declarations: [
    AppComponent,
    FeatureComponent
  ],
  imports: [
    BrowserModule,
    FormsModule,
    HttpModule,
    RouterModule.forRoot([
      { path: '', component: AppComponent,
        data: {
          meta: {
            title: 'App List',
            description: 'app ert5zuj gfdgh'
          }
        }
      },
      { path: 'feature', component: FeatureComponent,
        data: {
          meta: {
            title: 'Heroes List',
            description: 'test'
          }
        }
      },
      { path: '**', component: AppComponent }
    ]),
    MetaModule.forRoot(metaConfig) // MetaModule.forRoot() would also be possible here
  ],
  providers: [],
  bootstrap: [AppComponent]
})
export class AppModule { }

and here goes the app.components.ts:

import { Component } from '@angular/core';
import { MetaService } from 'ng2-meta';

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.css']
})
export class AppComponent {
  title = 'app works!';
  constructor(private metaService: MetaService) {}
}

I just tested it quickly and it seems to work. Would be cool if some more people could test it and then maybe we can release ng2-meta for angular 2 final and beyond πŸ‘
I have not tested if it's possible to set meta tags programmatically yet.

The

declare module 'ng2-meta';

in the typings.d.ts shouldn't be necessary anymore.

The Readme would have to be updated still etc.

@vinaygopinath I never encountered your issue, I encountered some others though. I think maybe you just forgot to import your RouterModule in your app.module.ts which then could result in the error when trying to importing RouterModule in meta.module.ts? Not sure though... Thank you also for the clarification on forRoot, I learnt a lot once again πŸ‘

bergben commented 8 years ago

If some guys could confirm that the branch is working we can realease the version for angular 2 final. You can try it in your project referencing the package in your package.json dependencies like so: [...] "ng2-meta": "git://github.com/vinaygopinath/ng2-meta.git#upgrade-to-angular2-final" "ng2-meta": "git://github.com/vinaygopinath/ng2-meta.git#feature/upgrade-to-angular2-final" [...]

@bencameron00 @vinaygopinath @bjornharvold @sunshineo

bjornharvold commented 8 years ago

Trying to test. That is not working for me.

"ng2-meta": "git://github.com/vinaygopinath/ng2-meta.git#upgrade-to-angular2-final",

npm ERR! git rev-list -n1 upgrade-to-angular2-final: fatal: ambiguous argument 'upgrade-to-angular2-final': unknown revision or path not in the working tree.
npm ERR! git rev-list -n1 upgrade-to-angular2-final: Use '--' to separate paths from revisions, like this:
npm ERR! git rev-list -n1 upgrade-to-angular2-final: 'git <command> [<revision>...] -- [<file>...]'
npm ERR! git rev-list -n1 upgrade-to-angular2-final: 
npm ERR! Darwin 16.0.0
npm ERR! argv "/usr/local/Cellar/node/6.6.0/bin/node" "/usr/local/bin/npm" "install"
npm ERR! node v6.6.0
npm ERR! npm  v3.10.8
npm ERR! code 128

npm ERR! Command failed: git rev-list -n1 upgrade-to-angular2-final
npm ERR! fatal: ambiguous argument 'upgrade-to-angular2-final': unknown revision or path not in the working tree.
npm ERR! Use '--' to separate paths from revisions, like this:
npm ERR! 'git <command> [<revision>...] -- [<file>...]'
npm ERR! 
npm ERR! 
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

npm ERR! Please include the following file with any support request:
bergben commented 8 years ago

I am sorry, I forgot the feature/ in the branch name. Please try again with [...] "ng2-meta": "git://github.com/vinaygopinath/ng2-meta.git#feature/upgrade-to-angular2-final" [...]

bjornharvold commented 8 years ago

I can get it to work.... however, I am having problem with MetaConfig.

I couldn't do:

import {MetaConfig, MetaModule} from 'ng2-meta';
let metaConfig = MetaConfig({
  useTitleSuffix: true,
  defaults: {
    title: 'Traveliko.com',
    titleSuffix: ' | Organic Hotel Booking - Always Travel with your Heart'
  }
});

I get this error:

ERROR in [default] /Users/crash/git/traveliko/traveliko-spa-frontend/src/app/app.module.ts:18:17 
Cannot find name 'MetaConfig'.

Had to do it like this for now:

let metaConfig = {
  useTitleSuffix: true,
  defaults: {
    title: 'Traveliko.com',
    titleSuffix: ' | Organic Hotel Booking - Always Travel with your Heart'
  }
};
bergben commented 8 years ago

Seems like for some reason the MetaConfig interface is not exported correctly. I asked about why that is on Gitter Angular, and StackOverflow here http://stackoverflow.com/questions/39977025/cannot-find-name-of-exported-interface?noredirect=1#comment67233040_39977025 .

@vinaygopinath Maybe you know more?

Otherwise I would just suggest to create the MetaConfig as @bjornharvold suggests, at least the functionality and everything seems to work in Angular 2 Final for me.

vinaygopinath commented 8 years ago

Hey folks, thanks a lot to everyone in this thread for helping out.

@bergben I tested your PR briefly and it works great with the route configuration.

@bjornharvold Since MetaConfig is an interface, you cannot initialize it with MetaConfig({..., as you noticed. Instead, you can annotate your config variable and make use of type checking.

let metaConfig: MetaConfig = {
  useTitleSuffix: true, // ok
  useSomethingElse: true // not ok, useSomethingElse is not defined in type MetaConfig
};

I'll check the dynamic meta changes functionality tonight - If everything checks out, I should be able to make minor changes, update the README and publish the new version tomorrow :)

Next up, server-side rendering/angular universal compatibility! :)

P.S: npm has a shorthand for github dependencies - you can install a lib directly from any Github repo with npm install --save user/projectname#branch-or-commit-sha as mentioned in this comment

bencameron00 commented 8 years ago

Sounds like great progress. When will the readme doc be updated to show us the new usage instructions?

vinaygopinath commented 8 years ago

README updated and new version published on npm! Thanks, everyone!

OdaiTu commented 7 years ago

thank you for this great work i have used the ng2-meta and when i click the inspect element it shows the meta good but when click on view page source nothing shown so the facebook and google cant read the page what is the solution ? i need this so bad so fast πŸ‘