Skip to content

docs: explain Types global namespace#292

Open
ProtonsAndElectrons wants to merge 3 commits into
piotrwitek:masterfrom
ProtonsAndElectrons:docs-types-global-namespace-97
Open

docs: explain Types global namespace#292
ProtonsAndElectrons wants to merge 3 commits into
piotrwitek:masterfrom
ProtonsAndElectrons:docs-types-global-namespace-97

Conversation

@ProtonsAndElectrons

Copy link
Copy Markdown

Fixes #97.

This adds a short section explaining the project-wide MyTypes ambient module, how feature areas extend it from the files that own those types, and how the typesafe-actions Types augmentation keeps RootAction available to helpers without re-declaring it in each feature.

Validation:

  • npm run ci-check
  • git diff --check

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new section titled 'Types Global Namespace' to the Redux documentation in both README.md and README_SOURCE.md. This section explains how to use an ambient module, MyTypes, to manage cross-cutting application types such as RootState and RootAction. Review feedback suggests improving consistency by using the tsx language tag for all code blocks, standardizing the import style for the MyTypes module, and removing unnecessary blank lines in code examples.

Comment thread README.md Outdated

This keeps the dependency direction simple. Feature modules export their own action, state, selector, and service types. The root store or service barrel composes those exports into application-wide types, and consumers import only the stable names they need:

```ts

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The language tag ts is used here, but the rest of the "Redux" section (including the block at line 1298 and the new block at line 1321) consistently uses tsx. For consistency across the guide, it is better to use tsx for all code blocks in this section.

Suggested change
```ts
```tsx

Comment thread README.md
This keeps the dependency direction simple. Feature modules export their own action, state, selector, and service types. The root store or service barrel composes those exports into application-wide types, and consumers import only the stable names they need:

```ts
import { RootAction, RootState, Services } from 'MyTypes';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This example introduces named imports from the MyTypes module (e.g., import { RootAction, ... }). However, existing examples throughout the guide (such as at lines 692 and 1803) use a default import style: import MyTypes from 'MyTypes'.

To maintain a consistent teaching pattern and avoid confusing readers, it would be beneficial to either update the existing examples to use named imports or explicitly mention that named imports are the preferred approach for this pattern.

Comment thread README.md
Comment on lines +1324 to +1325
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There is an unnecessary blank line at the end of this code block. Removing it will keep the formatting consistent with other blocks in the guide.

Suggested change
}
}

@ProtonsAndElectrons

Copy link
Copy Markdown
Author

Pushed dbf56bd to address the review feedback around the code fence language and import-style consistency. The generated service snippet still preserves its source-file trailing newline through generate-readme.js, so I left that generator behavior unchanged to keep this PR focused.

@ProtonsAndElectrons

Copy link
Copy Markdown
Author

Pushed a small follow-up in 1bcd2c8 to address the automated review nits: removed the trailing blank line from the generated code block and clarified that this section uses named imports while the existing namespace-style examples remain valid where grouped access is clearer.
pm run ci-check and the pre-push hook pass.

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.

Add a section about Types global namespace

1 participant