Skip to content

Commit

Permalink
feat: add dev error 4 non http(s) Twitter Card image URLs (#738)
Browse files Browse the repository at this point in the history
* feat: add dev error 4 non http(s) Twitter Card image URLs

* refactor: create util to warn about non http URLs

* feat: improve invalid URL message with relative URL comment
  • Loading branch information
davidlj95 authored Aug 3, 2024
1 parent 073700a commit 1ad2375
Show file tree
Hide file tree
Showing 11 changed files with 263 additions and 84 deletions.
13 changes: 13 additions & 0 deletions projects/ngx-meta/api-extractor/ngx-meta.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ export const __STANDARD_LOCALE_METADATA_SETTER_FACTORY: MetadataSetterFactory<St
// @internal (undocumented)
export const __STANDARD_TITLE_METADATA_SETTER_FACTORY: MetadataSetterFactory<Standard[typeof _GLOBAL_TITLE]>;

// @internal (undocumented)
export const __TWITTER_CARD_IMAGE_METADATA_SETTER_FACTORY: (metaService: NgxMetaMetaService) => (image: TwitterCard['image']) => void;

// @internal (undocumented)
export const _COMPOSED_KEY_VAL_META_DEFINITION_DEFAULT_SEPARATOR = ":";

Expand Down Expand Up @@ -147,6 +150,13 @@ export const makeMetadataManagerProviderFromSetterFactory: <T>(setterFactory: Me
// @internal (undocumented)
export const _makeMetadataResolverOptions: (jsonPath: MetadataResolverOptions['jsonPath'], global?: MetadataResolverOptions['global'], objectMerge?: MetadataResolverOptions['objectMerge']) => MetadataResolverOptions;

// @internal
export const _maybeNonHttpUrlDevMessage: (url?: string | URL, opts?: {
module?: string;
property?: string;
link?: string;
}) => void;

// @internal (undocumented)
class MetadataRegistry {
constructor(managers: ReadonlyArray<NgxMetaMetadataManager> | null);
Expand Down Expand Up @@ -264,6 +274,9 @@ export class NgxMetaStandardModule {
export class NgxMetaTwitterCardModule {
}

// @internal (undocumented)
export const _NO_OP: () => void;

// @public
export const OPEN_GRAPH_DESCRIPTION_METADATA_PROVIDER: FactoryProvider;

Expand Down
3 changes: 3 additions & 0 deletions projects/ngx-meta/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ export * from './src/ngx-meta-metadata-manager'
export * from './src/metadata-values'
export * from './src/ngx-meta.service'
export * from './src/ngx-meta-route-values.service'
// Internal utils
export * from './src/maybe-non-http-url-dev-message'
export * from './src/no-op'
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { _maybeNonHttpUrlDevMessage } from './maybe-non-http-url-dev-message'

describe('maybeNonHttpUrlDevMessage', () => {
const sut = _maybeNonHttpUrlDevMessage

beforeEach(() => {
spyOn(console, 'error')
})

describe('when URL is valid', () => {
// noinspection HttpUrlsUsage
const TEST_CASES = [
{ url: undefined, case: 'is not defined' },
{ url: 'http://example.com/image.jpg', case: 'uses HTTP protocol' },
{ url: 'https://example.com/image.jpg', case: 'uses HTTPS protocol' },
] as const
for (const testCase of TEST_CASES) {
describe(`like when URL ${testCase.case}`, () => {
it('should not emit any message', () => {
sut(testCase.url)

expect(console.error).not.toHaveBeenCalled()
})
})
}
})

describe('when URL is invalid', () => {
const TEST_CASES = [
{ url: 'assets/image.png', case: 'is relative' },
{
url: 'ftp://example.com/image.jpg',
case: 'uses a non-HTTP protocol (ie: ftp)',
},
] as const
for (const testCase of TEST_CASES) {
describe(`like when URL ${testCase.case}`, () => {
it('should emit a message about it', () => {
sut(testCase.url)

expect(console.error).toHaveBeenCalledWith(
jasmine.stringContaining(testCase.url),
)
})
})
}

it('should emit a message containing the provided extra options', () => {
let receivedMessage: string
;(console.error as jasmine.Spy).and.callFake(
(message) => (receivedMessage = message),
)
const opts = {
module: 'graphTweet',
property: 'profile image',
link: 'https://example.com/url-error',
} satisfies Parameters<typeof sut>[1]

sut(TEST_CASES[0].url, opts)

expect(receivedMessage!).toContain(opts.module)
expect(receivedMessage!).toContain(opts.property)
expect(receivedMessage!).toContain(opts.link)
})
})
})
26 changes: 26 additions & 0 deletions projects/ngx-meta/src/core/src/maybe-non-http-url-dev-message.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Logs an error message about a URL not being HTTP or HTTPs
*
* Useful to warn developers about some metadata that requires absolute HTTP
* or HTTPs URLs
*
* MUST be used with `ngDevMode` so that this message only runs in development
*
* @internal
*/
export const _maybeNonHttpUrlDevMessage = (
url?: string | URL,
opts: { module?: string; property?: string; link?: string } = {},
) => {
const urlStr = url?.toString()
if (!urlStr || urlStr.startsWith('http') || urlStr.startsWith('https')) {
return
}
const moduleStr = opts.module ? `/${opts.module}` : ''
const propertyStr = opts.property ? `${opts.property} ` : ''
const linkStr = opts.link ? `For more information, see ${opts.link}` : ''
console.error(
`ngx-meta${moduleStr}: ${propertyStr}URL must be absolute and use either http or https.\n` +
` -> Invalid ${propertyStr}URL: ${urlStr}\n${linkStr}`,
)
}
4 changes: 4 additions & 0 deletions projects/ngx-meta/src/core/src/no-op.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* @internal
*/
export const _NO_OP = () => {}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { OpenGraphMetadata } from './open-graph-metadata'
import { makeOpenGraphMetaDefinition } from './make-open-graph-meta-definition'

export const OPEN_GRAPH_KEY: keyof OpenGraphMetadata = 'openGraph'
export const OPEN_GRAPH_KEBAB_CASE_KEY = 'open-graph'

export const makeOpenGraphMetadataProvider = <Key extends keyof OpenGraph>(
key: Key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,69 +29,39 @@ describe('Open Graph image metadata', () => {

describe('setter', () => {
describe('when url is provided', () => {
describe('when the url does not start with http or https', () => {
it('should log an error to the console', () => {
spyOn(console, 'error')
it('should set all meta properties', () => {
sut(image)

const invalidImageUrl = 'ftp://ftp.example.com/images/og.jpg'
sut({ ...image, url: invalidImageUrl })

expect(console.error).toHaveBeenCalledWith(
jasmine.stringMatching(/http or https/),
invalidImageUrl,
)
})
})

describe('when the url is valid', () => {
it('should not error', () => {
spyOn(console, 'error')

sut(image)

expect(console.error).not.toHaveBeenCalled()
})
it('should set all meta properties', () => {
sut(image)

const props = Object.keys(image).length
expect(metaService.set).toHaveBeenCalledTimes(props)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.url,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.alt,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.secureUrl,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.type,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.width.toString(),
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.height.toString(),
)
})
const props = Object.keys(image).length
expect(metaService.set).toHaveBeenCalledTimes(props)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.url,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.alt,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.secureUrl,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.type,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.width.toString(),
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.height.toString(),
)
})
})
describe('when no url is defined', () => {
it('should not log any error', () => {
spyOn(console, 'error')

sut({ ...image, url: undefined })

expect(console.error).not.toHaveBeenCalled()
})

describe('when no url is defined', () => {
it('should remove all meta properties', () => {
sut({ ...image, url: undefined })

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { OpenGraph } from './open-graph'
import { _GLOBAL_IMAGE, NgxMetaMetaService } from '@davidlj95/ngx-meta/core'
import { makeOpenGraphMetadataProvider } from './make-open-graph-metadata-provider'
import {
_GLOBAL_IMAGE,
_maybeNonHttpUrlDevMessage,
NgxMetaMetaService,
} from '@davidlj95/ngx-meta/core'
import {
makeOpenGraphMetadataProvider,
OPEN_GRAPH_KEBAB_CASE_KEY,
} from './make-open-graph-metadata-provider'
import { makeOpenGraphMetaDefinition } from './make-open-graph-meta-definition'

const NO_KEY_VALUE: OpenGraph[typeof _GLOBAL_IMAGE] = {
Expand All @@ -21,18 +28,13 @@ export const __OPEN_GRAPH_IMAGE_SETTER_FACTORY =
const imageUrl = value?.url?.toString()
const effectiveValue: OpenGraph[typeof _GLOBAL_IMAGE] =
imageUrl !== undefined && imageUrl !== null ? value : NO_KEY_VALUE
//👇 What the f*ck? You may wonder (and with good cause https://youtu.be/U58IdBjMeS4?si=MT89dCrptOIS98Q9&t=27)
// Checkout https://github.com/davidlj95/ngx/pull/731 for an interesting rabbit hole about coverage reporting
// with `istanbul.js``, source maps & more fun
// noinspection HttpUrlsUsage
// Why not an `if`? Checkout https://github.com/davidlj95/ngx/pull/731
ngDevMode &&
imageUrl &&
!(imageUrl?.startsWith('http://') || imageUrl?.startsWith('https://')) &&
// prettier-ignore
console.error(
"ngx-meta/open-graph: an image URL must use either http or https.\n" +
"-> Invalid image URL: %s\n" +
"For more info, checkout https://stackoverflow.com/a/9858694/3263250", imageUrl)
_maybeNonHttpUrlDevMessage(imageUrl, {
module: OPEN_GRAPH_KEBAB_CASE_KEY,
property: _GLOBAL_IMAGE,
link: 'https://stackoverflow.com/a/9858694/3263250',
})
metaService.set(makeOpenGraphMetaDefinition(_GLOBAL_IMAGE), imageUrl)
metaService.set(
makeOpenGraphMetaDefinition(_GLOBAL_IMAGE, 'alt'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { TwitterCardMetadata } from './twitter-card-metadata'
import { makeTwitterCardMetaDefinition } from './make-twitter-card-meta-definition'

const TWITTER_KEY: keyof TwitterCardMetadata = `twitterCard`
export const TWITTER_KEY_KEBAB_CASE = 'twitter-card'

export const makeTwitterCardMetadataProvider = <Key extends keyof TwitterCard>(
key: Key,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { enableAutoSpy } from '@/ngx-meta/test/enable-auto-spy'
import { MetadataSetter, NgxMetaMetaService } from '../../core'
import { TestBed } from '@angular/core/testing'
import { MockProviders } from 'ng-mocks'
import { TwitterCard } from './twitter-card'
import {
__TWITTER_CARD_IMAGE_METADATA_SETTER_FACTORY,
TWITTER_CARD_IMAGE_METADATA_PROVIDER,
} from './twitter-card-image-metadata-provider'
import { TwitterCardImage } from './twitter-card-image'

describe('Twitter Card image metadata', () => {
enableAutoSpy()
let sut: MetadataSetter<TwitterCard['image']>
let metaService: jasmine.SpyObj<NgxMetaMetaService>

beforeEach(() => {
sut = makeSut()
metaService = TestBed.inject(
NgxMetaMetaService,
) as jasmine.SpyObj<NgxMetaMetaService>
})

const image = {
url: 'https://example.com/foo.png',
alt: 'Alternative text',
} satisfies TwitterCardImage

describe('when image is provided', () => {
it('should set all meta properties', () => {
// noinspection DuplicatedCode
sut(image)

const props = Object.keys(image).length
expect(metaService.set).toHaveBeenCalledTimes(props)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.url,
)
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
image.alt,
)
})
})

describe('when no image provided', () => {
it('should remove all meta properties', () => {
sut(undefined)

const props = Object.keys(image).length
expect(metaService.set).toHaveBeenCalledTimes(props)
for (let i = 0; i < props; i++) {
expect(metaService.set).toHaveBeenCalledWith(
jasmine.anything(),
undefined,
)
}
})
})
})

function makeSut(): MetadataSetter<TwitterCard['image']> {
TestBed.configureTestingModule({
providers: [
MockProviders(NgxMetaMetaService),
TWITTER_CARD_IMAGE_METADATA_PROVIDER,
],
})
return __TWITTER_CARD_IMAGE_METADATA_SETTER_FACTORY(
TestBed.inject(NgxMetaMetaService),
)
}
Loading

0 comments on commit 1ad2375

Please sign in to comment.