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.

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

Jump to bottom

fix: icons, interactionsTests, testIds types exports #1369

Merged
merged 7 commits into from
Jun 28, 2023
Merged

fix: icons, interactionsTests, testIds types exports #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1b094a3
Remove unrelevant comments
SergeyRoyt Jun 13, 2023
c7fac38
Merge branch 'master' into fix/sergeyro/icon-export
SergeyRoyt Jun 15, 2023
7b2c43f
Merge branch 'master' into fix/sergeyro/icon-export
SergeyRoyt Jun 25, 2023
b99dd38
fix: <Text> in published-ts-components.js
SergeyRoyt Jun 25, 2023
71c2a30
Refactoring
SergeyRoyt Jun 25, 2023
248f51f
Fix icons, interactionsTests, testIds types export via typesVersions
SergeyRoyt Jun 25, 2023
416097a
Merge branch 'master' into fix/sergeyro/types-exports
SergeyRoyt Jun 25, 2023
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
17 changes: 15 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,27 @@
},
"./dist/": "./dist/",
"./icons": "./dist/esm/src/components/Icon/Icons/icons.js",
"./tokens": "./dist/tokens/tokens.css",
"./storybookComponents": "./dist/storybook/index.js",
"./interactionsTests": "./dist/esm/src/tests/interactionsTests.js",
"./testIds": "./dist/esm/src/tests/testIds.js",
"./storybookComponents": "./dist/storybook/index.js",
"./tokens": "./dist/tokens/tokens.css",
"./mockedClassNames": {
"import": "./dist/mocked_classnames_esm/src/index.js"
}
},
"typesVersions": {
"*": {
"icons": [
"./dist/esm/components/Icon/Icons/index.d.ts"
],
"interactionsTests": [
"./dist/esm/tests/interactions-utils.d.ts"
],
"testIds": [
"./dist/esm/tests/test-ids-utils.d.ts"
]
Comment on lines +28 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these two needed to be publically exposed APIs?
The interactionsTests are mostly wrappers from regular DOM functions and they are very platform specific

Copy link
Contributor Author

Choose a reason for hiding this comment

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

testIds are containing ComponentDefaultTestId which contains all default data-testid for components and it's being used already in the monolith in cypress tests selectors
image

interactionsTests are exporting functions which helps to build storybook interaction tests and this endpoint is being used in the monday-ui-components rn
image

Also third one should be storybookComponents to export components we are using to build our storybook, but as they are not converted to ts, they are not here

Copy link
Contributor

Choose a reason for hiding this comment

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

ok. I'm not too fond of it as a public API. it creates a coupling between Vibe's internal testing tools with others who consume it, making it harder to refactor. if that's the current state, then fine, but let's try to reduce such dependencies

}
},
"scripts": {
"test": "jest",
"test:stories": "testing=storybook TEST_END_FILES=jest-story jest",
Expand Down
2 changes: 0 additions & 2 deletions scripts/build-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,4 @@
const { createFoldersIfNotExist, buildComponentsTypesIndexFile } = require("./build-utils");
createFoldersIfNotExist();

// we create new d.ts index file but still not give reference to it in our package.json file
// until we finish to migrate all the components to typescript
buildComponentsTypesIndexFile();
4 changes: 2 additions & 2 deletions scripts/build-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ function buildComponentsTypesIndexFile() {
buildComponentExport(name, `./types/${path}`)
);

const exportsWithJavasript = Object.keys(publishedJSComponents).map(name => buildExportToComponentWithoutType(name));
convertExportsToFile(exportsWithTypescript.concat(exportsWithJavasript), "types.d.ts");
const exportsWithJavascript = Object.keys(publishedJSComponents).map(name => buildExportToComponentWithoutType(name));
convertExportsToFile(exportsWithTypescript.concat(exportsWithJavascript), "types.d.ts");
}

function buildStorybookComponentsIndexFile() {
Expand Down
2 changes: 0 additions & 2 deletions webpack/published-ts-components.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ const publishedTSComponents = {
useActiveDescendantListFocus: "hooks/useActiveDescendantListFocus",
useListenFocusTriggers: "hooks/useListenFocusTriggers",
useSwitch: "hooks/useSwitch",
// Don't remove next line
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty sure
We have this marker on the line 87

// plop_marker:published-hooks
useClickableProps: "hooks/useClickableProps/useClickableProps",
useHover: "hooks/useHover/useHover"
};
Expand Down