Skip to content

feat(flow-item): expose FlowItemLike type #11791

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

Merged
merged 8 commits into from
Mar 26, 2025
Merged

feat(flow-item): expose FlowItemLike type #11791

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2331c82
feat(flow-item): expose FlowItemLike type from root module
benelan Mar 21, 2025
1efeb9d
Merge branch 'dev' into benelan/11750-expose-flow-item-like-type
benelan Mar 21, 2025
01ea7c9
chore: cleanup
benelan Mar 22, 2025
d924bfe
feat: expose IconName type
benelan Mar 22, 2025
64b344c
chore: add issue number to comment for context
benelan Mar 24, 2025
c9bef4b
Merge remote-tracking branch 'origin/dev' into benelan/11750-expose-f…
benelan Mar 25, 2025
4602eb4
refactor: move publicly exposed types to component dir
benelan Mar 26, 2025
19d018b
chore: cleanup
benelan Mar 26, 2025
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
4 changes: 4 additions & 0 deletions packages/calcite-components/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ export const setAssetPath: Runtime["setAssetPath"] = (path) => {
};

export { getAssetPath } from "./runtime";

// Expose specific development types below (by stakeholder request). Context: #10763
export type { FlowItemLike } from "./components/flow/interfaces";
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way we could expose these utility interfaces through the types output directory (e.g., types/components/flow/, types/utils/components/flow/ or something along those lines)? I’d prefer to keep the interfaces close to their associated component rather than mixing them all into index.ts. cc @maxpatiiuk

Not sure if we would also to split public and internal interfaces into separate files.

Copy link
Member

@maxpatiiuk maxpatiiuk Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

sure:

Create a file like src/types.ts (To allow importing from "@esri/calcite-components/types")
Then in your package.json add the following to exports map

  "exports": {
    ".": "./dist/index.js",
    "./loader": "./dist/loader.js",
    "./package.json": "./package.json",
    "./components/*/customElement": "./dist/components/*/customElement.js",
    "./components/*": "./dist/components/*/index.js",
+  "./types": "./dist/types.d.ts",
    "./types/*": "./dist/types/*.d.ts",
    "./hydrate": "./hydrate/index.js",
    "./calcite/calcite.css": "./dist/calcite/calcite.css",
    "./docs/*": "./dist/docs/*",
    "./dist/calcite/calcite.css": "./dist/calcite/calcite.css",
    "./dist/loader": "./dist/loader.js",
    "./dist/components": "./dist/index.js",
    "./dist/components/*": "./dist/components/*/index.js"
  },

or 2:
Create a file like src/components/flow/types.ts (To allow importing from "@esri/calcite-components/components/calcite-flow/types")
Then in your package.json add the following to exports map

  "exports": {
    ".": "./dist/index.js",
    "./loader": "./dist/loader.js",
    "./package.json": "./package.json",
    "./components/*/customElement": "./dist/components/*/customElement.js",
+  "./components/calcite-flow/types": "./dist/components/calcite-flow/types.d.ts",
    "./components/*": "./dist/components/*/index.js",
    "./types/*": "./dist/types/*.d.ts",
    "./hydrate": "./hydrate/index.js",
    "./calcite/calcite.css": "./dist/calcite/calcite.css",
    "./docs/*": "./dist/docs/*",
    "./dist/calcite/calcite.css": "./dist/calcite/calcite.css",
    "./dist/loader": "./dist/loader.js",
    "./dist/components": "./dist/index.js",
    "./dist/components/*": "./dist/components/*/index.js"
  },

or 3:

Create a file like src/types/components/calcite-flow.ts (To allow importing from "@esri/calcite-components/types/components/calcite-flow")
No need for any changes in package.json

or 4:

Create a file like src/components/flow/types/index.ts (To allow importing from "@esri/calcite-components/components/calcite-flow/types")
No need for any changes in package.json

or 5:
export that types you need directly from calcite-flow.tsx instead of creating any separate files

4th and 5th options are probably my favorite

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for sharing, @maxpatiiuk! I really dig option 4 since it aligns with how we structure our component files and makes it clear those exports are meant for public consumption. WDYT, @benelan?

Copy link
Member Author

@benelan benelan Mar 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I'd like number 5 if importing the named type also triggers side effects, because then it would only be one import statement instead of two.

Option 5 assuming the above:

import type { FlowItemLike } from "@esri/calcite-components/dist/components/calcite-flow"

Option 4:

import "@esri/calcite-components/dist/components/calcite-flow"
import type { FlowItemLike } from "@esri/calcite-components/dist/components/calcite-flow/types"

If option 5 would require the following, then I'd also choose option 4:

import "@esri/calcite-components/dist/components/calcite-flow"
import type { FlowItemLike } from "@esri/calcite-components/dist/components/calcite-flow"

I can do some testing, unless either of you know off the top of your head.

Copy link
Member

Choose a reason for hiding this comment

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

I believe the side-effect will only take place if the import is emitted, so just a type import will break that.

Copy link
Member

Choose a reason for hiding this comment

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

https://www.typescriptlang.org/docs/handbook/modules/reference.html#type-only-imports-and-exports

Copy link
Member

@maxpatiiuk maxpatiiuk Mar 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Good point about side-effect - the following should work though:

import { type FlowItemLike } from "@esri/calcite-components/components/calcite-flow"

While the above is not a concern if we export a non-type-only thing, that would create concerns around HMR when you are developing components - As exporting anything other than component from a file disables HMR

despite that, this option has nice benefits:

  • it this much more discoverable, especially because, at least in map components for now, we display the import path at the top of the reference page
    Screenshot 2025-03-25 at 17 39 14

    • In comparison, It is hard to know that a /types entry point exist for some components without examining the dist/ folder

  • One less file to create thus less work for developers

Note that whatever pattern you go with is the pattern I would encourage other Lumina packages to follow

maybe go with option 4 for now but improve the way it is represented in the documentation in the future?

Copy link
Member

Choose a reason for hiding this comment

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

In comparison, It is hard to know that a /types entry point exist for some components without examining the dist/ folder

Agreed. Though I guess the component’s own exports would also need to be explored if they are not documented, right?

One less file to create thus less work for developers

I see this as a benefit since the separate file/extra steps required to expose an interface could help avoid unintentional sharing.

maybe go always option for for now but improve the way it is represented in the documentation in the future?

For sure. Maybe we can wait for more use cases before adding this to the doc since FlowItemLike is more of an advanced, internal use case at the moment.

export type { IconName } from "./components/icon/interfaces";
Copy link
Member

Choose a reason for hiding this comment

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

Let's hold back on this. Unlike FlowItemLike, this interface is available through the icon property†.

†icon prop type is also unioned with string (to be removed at a later stage).