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

refactor: Use the module name rather than naming all of the exported packages ".../types" #1866

Open
8 tasks
Unique-Divine opened this issue May 7, 2024 · 1 comment · May be fixed by #2199
Open
8 tasks
Assignees
Labels
epic Project or large task that groups multiple tickets and PRs

Comments

@Unique-Divine
Copy link
Member

Unique-Divine commented May 7, 2024

Context

In older Cosmos-SDK versions, it was common to import the publicly exposed types
from a module by adding a "types" suffix to the package name. For example,

import (
  banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
  devgastypes "github.com/NibiruChain/nibiru/x/devgas/v1/types"
)

Task List

[Expand Task Menu]

Implications

  1. This means that the true package name, "types", has to be overwritten with an
    alias for each import. The Golang LSP (what your IDE understands) still considers
    all of the packages as having the name "types", so your IDE will only suggest
    variables if you start typing "types". You can't just type "banktypes" in a new
    file and see code suggestions.

  2. When you use types from two modules at the same time or read code from within
    a module, you'll see a nonsemantic prefix, "types" everywhere. Essentially,
    package namespacing becomes irrelevant with this approach.

Suggestion

Follow the pattern of the nibiru/eth and nibiru/x/evm packages (also the
convention from v0.50 of the Cosmos-SDK) by using the top-level package of a
module for its types.

In other words, use bank.MsgSend instead of types.MsgSend or
banktypes.MsgSend.

Support Points:

  1. Less namespace collisions: The module namespace is often completely unused aside from
    one place, which is the top-level "app" directory.

  2. IDEs become more useful: You can type a module's name and have your IDE automatically suggest its types

  3. Less aliasing: Using an alias for every import produces a bad code smell (opinionated take).

"Using a generic name makes it easy to accidentally use the wrong value in
a different file" - [Uber Golang Style Guide]

I'd argue that this argument holds for imports and not just for other
variables.

@Unique-Divine Unique-Divine added epic Project or large task that groups multiple tickets and PRs type: refactor labels May 7, 2024
@expertdicer expertdicer self-assigned this Jan 21, 2025
@expertdicer expertdicer moved this to ➡️ Pending Review/Merge in ⚛️ Nibiru (Hougyoku) Feb 5, 2025
@expertdicer expertdicer moved this from ➡️ Pending Review/Merge to ⚡ Building 🧱 in ⚛️ Nibiru (Hougyoku) Feb 5, 2025
expertdicer added a commit that referenced this issue Feb 6, 2025
@expertdicer expertdicer linked a pull request Feb 6, 2025 that will close this issue
@Unique-Divine
Copy link
Member Author

Further Clarification: #2199 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Project or large task that groups multiple tickets and PRs
Projects
Status: ⚡ Building 🧱
Development

Successfully merging a pull request may close this issue.

3 participants