Skip to content

Commit

Permalink
refresh developer guidelines
Browse files Browse the repository at this point in the history
  • Loading branch information
himdel committed Jun 19, 2024
1 parent d6034c9 commit c0ed373
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 29 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

Frontend for Ansible Hub and Galaxy. The backend for this project can be found at [ansible/galaxy\_ng](https://github.com/ansible/galaxy_ng/),
developer docs at [ansible.readthedocs.io](https://ansible.readthedocs.io/projects/galaxy-ng/en/latest/), and an outdated wiki at [ansibe/galaxy\_ng wiki](https://github.com/ansible/galaxy_ng/wiki/Development-Setup).
Also read [developer\_guidelines](./developer_guidelines.md).

The project is built on React & Patternfly, using components from [patternfly-react](https://github.com/patternfly/patternfly-react), with [lingui](https://github.com/lingui/js-lingui/) for l10n.

Expand Down
77 changes: 48 additions & 29 deletions developer_guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@

These are the rules we try to follow to make sure this project is as consistent as possible


## Imports

Imports get automatically sorted by prettier. For local imports, make sure to import from
`src/SUBDIR` without the full path, or `./FILE`. Only use the file extension for scss &
images.
Imports get automatically sorted by prettier. For local imports, make sure to import from `src/SUBDIR` without the full path, or `./FILE`.
Only use the extension for scss & images.

All components in local directories should be exported and imported via the index.ts file
at the directory's root.
All components in local directories should be exported and imported via the `index.ts` file at the directory's root.

Correct way to import:

Expand All @@ -20,24 +19,21 @@ import { CollectionList, PartnerHeader } from 'src/components';
Incorrect way to import:

```
import { CollectionList } from 'src/components/collection-list/collection-list'
import { CollectionList } from '../collection-list/collection-list'
import { PartnerHeader } from 'src/components/headers/partner-header'
```

Not only is this a lot cleaner to read and update, but it also makes it easier to reorganize
code without breaking imports.
This is configured in `.prettierrc.yaml` using the `@trivago/prettier-plugin-sort-imports`
prettier plugin and the `importOrder*` configs.


## Page Query Params

The state of a page should largely be dictated by the query parameter for that page when possible.
This means that a page's components such as pagination, tabs, filter's etc. should derive their
values from the page's query parameters as well as update the query params when they change.
The state of a page should largely be dictated by the query parameter for that page when possible. This means that components such as pagination, tabs, filters etc. should derive their values from the page's query parameters as well as update the query params when they change.

This is done by storing a `params` object in the component state. This object contains
keywords for each parameter in they query. Params with single values are stored as a single
value and params with multiple values are stored as an array. As an example
This is done by storing a `params` object in the component state. This object contains keywords for each parameter in they query. Params with single values are stored as a single value and params with multiple values are stored as an array.

`?key1=hi&key1=bye&key2=world` is represented as
As an example `?key1=hi&key1=bye&key2=world` is represented as

```
this.state.params = {
Expand All @@ -46,27 +42,23 @@ this.state.params = {
}
```

Updating the params keyword should be done with the `ParamHelper` object found in
`src/utilities/param-helper`. This object contains a set of pure functions that can
be used to update `params` as well as mixins that can be used to update the pages's
query params when the state changes.
Updating the params keyword should be done with `ParamHelper` from `src/utilities`.
This object contains a set of pure functions that can be used to update `params` as well as mixins that can be used to update query params when the state changes.

An example of how this works is the `Sort` component (`src/components/sort.tsx`).
This component loads the field being sorted and direction to sort by from `params['sort']`.
When the component is changed it calls the `updateParams` callback which updates the component's
`params` object, the page's query params and optionally calls the API with the new params to
update the data being displayed.
This component loads the field being sorted and direction to sort by from `params.sort`.
When the component is changed, it calls the `updateParams` callback which updates the component's `params` object, the page's query params and optionally calls the API with the new params to update the data being displayed.

Generally speaking a page's query parameters should match the API's query parameters when possible.

An exception to this are the `sort`, `page` and `page_size` params - we always use these in the query string and page state, but the API layer will translate them to `ordering`, `order_by`, `offset` and `limit` where appropriate.

Generally speaking a page's query parameters should match the API's query parameters when
possible.
Following these rules will ensure that specific page's state can be bookmarked, shared, and retained after they are reloaded.

Following these rules will ensure that specific page's state can be bookmarked, shared,
and retained after they are reloaded.

## URLs

**NEVER** hardcode URLs to any paths within automation hub. This makes it very difficult
to update the app's routing later on.
**Never** hardcode URLs to any paths within automation hub. This makes it very difficult to update the app's routing later on.

Instead use the `formatPath` function to get URLs, with paths from the `Paths` enum.
Both of these are found in `src/paths.tsx`.
Expand All @@ -83,3 +75,30 @@ Example:
// With ?params
<Link to={formatPath(Paths.myNamespaces, {}, { page: 1 })} />
```

Given a path like `Paths.foo = '/foo/:bar'`, `formatPath(Paths.foo)` will fail because the value of `:bar` was not provided, `formatPath(Paths.foo, { bar: 'abc' })` will yield `<PREFIX>/foo/abc`. `<PREFIX>` is an empty string in standalone/community modes, but in insights mode, it becomes `/ansible/automation-hub`.

The insights mode router uses a basename of `/`, `/beta/`, or `/preview/`, with hub mounted under `ansible/automation-hub`,
while the standalone mode uses a basename of `UI_BASE_PATH` (typically `/ui/`), with hub mounted directly there.

In standalone mode, the router doesn't handle anything outside `/ui/`, and code in `src/entry-standalone.tsx` takes care of redirecting to `/ui/`,
or to `/ui/dispatch/` when the url matches a `/namespace/collection`-like format.

Use `Link to=` with URLs from `formatPath` (relative to router base path), and `ExternalLink href=` for real-word URLs going outside hub.
`ExternalLink` also takes care of setting the right `rel=nofollow`, etc. attributes and adding an external link icon.


## Linters

`npm run lint` runs all the linters, `npm run lint-fix` runs all the linters which can auto-fix things. `npm run lint-setup` tries to install any missing linters.


## Helper tasks

`find-unused-exports`: tries to find dead exports, with some false positives. Needs `npm run imports-to-relative` to work.
`gettext:compile`: convert `locale/*.po` files to JSON, save to `locale/*.json`.
`gettext:extract`: extract all translatable strings from `src/` to `locale/*.po`.
`imports-to-relative`: changes all imports from `src/foo` to `../foo` or `../../foo` etc., depending on depth.
`imports-to-src`: changes all imports from `../foo` or `../../foo` etc. to `src/foo`. Opposite of `imports-to-relative`.
`prettier`: fix prettier issues
`sort-exports`: sort `src/*/index.ts` exports - changes export to import, runs prettier, changes back

0 comments on commit c0ed373

Please sign in to comment.