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.

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

Improve examples generation #947

Merged
merged 2 commits into from
Jul 4, 2024
Merged

Conversation

j3rem1e
Copy link
Contributor

@j3rem1e j3rem1e commented Aug 12, 2023

  • Generate "additionalProperties" examples. Support the vendor extension "x-additionalPropertiesName"
  • Use the default value if provided
  • Generate null instead of a (potentially) invalid string if the type is not found (same behavior here as redoc and others)

- Generate "additionalProperties" examples. Support the vendor extension "x-additionalPropertiesName"
- Use the default value if provided
- Generate null instead of a (potentially) invalid string if the type is not found (same behavior here as redoc and others)
Copy link

@llllvvuu llllvvuu left a comment

Choose a reason for hiding this comment

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

Thanks for the regex feature! Only nit is that "regex-to-strings" is not a very popular package (18 stars / 4150 weekly downloads), the bundle size is huge (152 KB minified and treeshaked), and the output is random.

// test.ts:
import { expandN } from 'regex-to-strings';

const DATE_REGEX = "(\\d{4}-[01]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d:[0-5]\\d\\.\\d+([+-][0-2]\\d:[0-5]\\d|Z))|(\\d{4}-[01]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d:[0-5]\\d([+-][0-2]\\d:[0-5]\\d|Z))|(\\d{4}-[01]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d([+-][0-2]\\d:[0-5]\\d|Z))"
const REGEX = DATE_REGEX.replace(/[+*](?![^\][]*[\]])/g, '{8}').replace(/\{\d*,(\d+)?\}/g, '{8}')
console.log(expandN(REGEX, 1)[0])
; tsx test.ts
1699-00-12T10:55:27.89814068+24:18
; npm run bundle
...
CJS dist/test.js 151.01 KB

I think deterministic randExp.js as suggested in #951 is preferable:

// test.ts
import RandExp from "randexp"
RandExp.prototype.randInt = (from, to) => from

const DATE_REGEX = "(\\d{4}-[01]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d:[0-5]\\d\\.\\d+([+-][0-2]\\d:[0-5]\\d|Z))|(\\d{4}-[01]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d:[0-5]\\d([+-][0-2]\\d:[0-5]\\d|Z))|(\\d{4}-[01]\\d-[0-3]\\dT[0-2]\\d:[0-5]\\d([+-][0-2]\\d:[0-5]\\d|Z))"
console.log(new RandExp(DATE_REGEX).gen())
; tsx test.ts
0000-00-00T00:00:00.0+00:00
; npm run bundle
...
CJS dist/test.js 9.55 KB

It's also more well-tested with 1.8k stars and 3,104,261 weekly downloads

@@ -257,7 +262,14 @@ export function getSampleValueByType(schemaObj) {
if (typeValue.match(/^string/g)) {
if (schemaObj.enum) { return schemaObj.enum[0]; }
if (schemaObj.const) { return schemaObj.const; }
if (schemaObj.pattern) { return schemaObj.pattern; }
if (schemaObj.pattern) {
const examplePattern = schemaObj.pattern.replace(/[+*](?![^\][]*[\]])/g, '{8}').replace(/\{\d*,(\d+)?\}/g, '{8}');

Choose a reason for hiding this comment

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

nit: this line would be unnecessary with randexp.js, as randexp will just put the minimum number of characters (1), which is cleaner than 8 IMO

@j3rem1e
Copy link
Contributor Author

j3rem1e commented Aug 19, 2023

Honestly, I totally agree with you, but I tried to keep the compatibility with a project which looks like to share the same codebase (openapi-explorer) and uses this lib.

I`ll will change the PR however to use the lib you propose,

@llllvvuu
Copy link

share the same codebase (openapi-explorer) and uses this lib

Wow, I didn't know there was a fork! I just made a PR for them to switch the lib too: Authress-Engineering/openapi-explorer#180 and mention rapidoc attribution: Authress-Engineering/openapi-explorer#181

@mrin9
Copy link
Collaborator

mrin9 commented Jul 4, 2024

@j3rem1e Thanks for the PR. I am sorry for the delay in accepting the PR. Our team has started to resume work on this library

@mrin9 mrin9 merged commit 058183f into rapi-doc:master Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants