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: add option to override react config. #254

Closed
wants to merge 4 commits into from
Closed

feat: add option to override react config. #254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1499e4
Add option to override react config.
btimby Aug 24, 2021
21b70c7
Add unit test and update docs.
btimby Aug 24, 2021
5208a78
Fix table formatting.
btimby Aug 24, 2021
5921718
Remove space in stringified object.
btimby Aug 24, 2021
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ npm install -g @asyncapi/generator
| singleFile | Set output into one html-file with styles and scripts inside | No | `false` | `true`,`false` | `true` |
| outFilename | The filename of the output file. | No | `index.html` | *Any* | `asyncapi.html` |
| pdf | Generates output HTML as PDF | No | `false` | `true,false` | `false` |
| config | Stringified JS object containing options for react | No | null | *Any* | {"show":{"sidebar":false}} |
Copy link
Member

Choose a reason for hiding this comment

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

Please add link to the React component's config shape, something like:

Suggested change
| config | Stringified JS object containing options for react | No | null | *Any* | {"show":{"sidebar":false}} |
| config | Stringified JS object containing [options for the React component](https://github.com/asyncapi/asyncapi-react/blob/next/docs/configuration/config-modification.md#definition) | No | null | *Any* | {"show":{"sidebar":false}} |


> **NOTE**: If you only generate an HTML website, set the environment variable `PUPPETEER_SKIP_CHROMIUM_DOWNLOAD` to `true` and the generator will skip downloading chromium.

Expand Down
5 changes: 4 additions & 1 deletion filters/all.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ const filter = module.exports;
* Prepares configuration for component.
*/
function prepareConfiguration(params = {}) {
const config = { show: { sidebar: true }, sidebar: { showOperations: 'byDefault' } };
let config = { show: { sidebar: true }, sidebar: { showOperations: 'byDefault' } };
if (params.config) {
config = {...config, ...JSON.parse(params.config) };
}
Comment on lines +13 to +16
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should merge the show part of config with the default. In your proposition if someone pass as config:

{ "show": { "operations": false } }

Then operations will be hidden, but sidebar will also hidden, but by default should be showed - only explicit defined sidebar: false should hide the sidebar, so please change it something like (maybe you will come up with something more better):

Suggested change
let config = { show: { sidebar: true }, sidebar: { showOperations: 'byDefault' } };
if (params.config) {
config = {...config, ...JSON.parse(params.config) };
}
let config = { show: { sidebar: true }, sidebar: { showOperations: 'byDefault' } };
if (params.config) {
const parsedCustomConfig = JSON.parse(params.config);
config = {...config, ...parsedCustomConfig, show: { ...config.show, ...(parsedCustomConfig.show || {}) } };
}

if (params.sidebarOrganization === 'byTags') {
config.sidebar.showOperations = 'bySpecTags';
}
Expand Down
5 changes: 5 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@
"pdf": {
"description": "Set to `true` to get index.pdf generated next to your index.html",
"default": false
},
"config": {
"description": "Stringified JS object containing options for react, ex: {\"show\":{\"sidebar\":false}}.",
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the description from my suggestion.

"default": null,
"required": false
}
},
"generator": ">=1.7.3 <2.0.0",
Expand Down
8 changes: 8 additions & 0 deletions test/filters/all.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,14 @@ describe('Filters', () => {
const result = stringifyConfiguration(params);
expect(result).toEqual(expected);
});

it('should allow override of config', async () => {
const params = { config: '{"show": {"sidebar":false}}' };
const expected = '{"show":{"sidebar":false},"sidebar":{"showOperations":"byDefault"}}';

const result = stringifyConfiguration(params);
expect(result).toEqual(expected);
});
Comment on lines +116 to +122
Copy link
Member

Choose a reason for hiding this comment

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

Please also add one test with checking my proposition with deep merging of the config prop :)

});

describe('.renderSpec', () => {
Expand Down