ni / ni/nimble

Organize repo to consistently have an import per component

Open
#172 14 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

tech debt
Dominant language
TypeScript
Stars
39
Forks
14
Avg merge
1d 16h
Merged PRs (30d)
13

Description

Currently nimble-components and nimble-angular organize each component into a separate directory with separate import strategies.

Edit August 2023:

The current recommendation would be as follows (these are not coupled, we could do the smaller nimble-components work first):

  • nimble-components:
    • Switch to using npm package entry points
    • Leveraging package entrypoints, elide the dist/esm from import paths so for instance a button import changes from import '@ni/nimble-components/dist/esm/button'; to import '@ni/nimble-components/button';
    • Should decide how we migrate to es module resolution within the package. i.e. showing the full extension in the import path.
    • Should decide what is private and what is public apis in nimble-components when using package exports.
      • Could potentially stage the private/public enforcement before the rename to remove dist/esm. This could help avoid bugs like we fixed in #2290
  • nimble-angular:
    • Switch to secondary entrypoints per directive. Note: we should profile build performance before going down this road. We might also want to be on a nearly-new version of Angular to get modern performance numbers.
    • Switch to Angular single file / standalone component pattern for our directives, pipes, etc.
    • Leveraging secondary entrypoints and single file components an angular app can avoid needing to import a Module per component and would import a button directive from @ni/nimble-angular/button
  • Go through the comments in the thread and make sure they are addressed

All libraries:

  • When we get to make the big breaking path changes we should update the icon paths from the plural icons/add to i.e. just icon/add. This reflects our other path conventions to make the path align with the icon name. (Realizing years later I missed that 🤦)

Known issues this change will resolve:

  • In nimble-angular today the vast majority of components are exported together from the top-level primary entrypoint. Any import at that level results in all components from that entry point being registered and defined in a page: https://github.com/ni/nimble/issues/1476

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by comparing the current component layouts and import strategies in nimble-components and nimble-angular, then review the comments in the issue thread. Profile Angular build performance before evaluating secondary entrypoints. Done means the agreed public import paths, package exports, Angular entrypoints, and icon path conventions are implemented and the known import-registration issue is addressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
angular, node.js, typescript
Domain
build-system, frontend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.