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.

Sign up for GitHub

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

feat(helpers): add support for complex intermediate return types in fake method #2381

Closed
wants to merge 1 commit into from
Closed

feat(helpers): add support for complex intermediate return types in fake method #2381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/modules/helpers/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,7 @@ export class HelpersModule {

// split the method into module and function
const parts = method.split('.');
const property = parts.length === 3 ? parts.pop() : undefined;

let currentModuleOrMethod: unknown = this.faker;
let currentDefinitions: unknown = this.faker.rawDefinitions;
Expand Down Expand Up @@ -1231,7 +1232,11 @@ export class HelpersModule {
params = [parameters];
}

const result = String(fn(...params));
const fnResult = fn(...params);
const result =
typeof fnResult === 'object' && property
? String(fnResult[property as keyof typeof fnResult])
: String(fnResult);

// Replace the found tag with the returned fake value
// We cannot use string.replace here because the result might contain evaluated characters
Expand Down
23 changes: 23 additions & 0 deletions test/modules/helpers.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,29 @@ describe('helpers', () => {
it('should not trim whitespace', () => {
expect(faker.helpers.fake(' --- ')).toBe(' --- ');
});

it('should be able to handle methods that return a complex object', () => {
expect(
faker.definitions.airline.airport.map((airport) => airport.iataCode)
).toContain(faker.helpers.fake('{{airline.airport.iataCode}}'));
});

it('should return undefined if a method returns a complex object but the property is undefined', () => {
expect(faker.helpers.fake('{{airline.airport.code}}')).toBe(
Copy link
Member

Choose a reason for hiding this comment

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

Currently if a property is unknown on we receive an error:

Invalid module method or definition: person.abc
- faker.person.abc is not a function
- faker.definitions.person.abc is not an array

I would prefer if we could throw this error here as well and add a case for faker.person is a function but does not return an object with the property abc. I'll gladly take suggestions on the phrasing.

'undefined'
);
});

it('should be able to pass multiple dynamic templates with complex objects', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this test actually adds any value since we already have a test for an array argument:

faker/test/modules/helpers.spec.ts

Lines 1082 to 1089 in 604d52d

it('should be able to pass multiple dynamic templates', () => {
expect(faker.definitions.location.city_name).toContain(
faker.helpers.fake([
'{{location.city_name}}',
'{{location.cityName}}',
])
);
});

expect(
faker.definitions.airline.airplane.map((airplane) => airplane.name)
).toContain(
faker.helpers.fake([
'{{airline.airplane.name}}',
'{{airline.airplane.name}}',
])
);
});
});

describe('rangeToNumber()', () => {
Expand Down