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

Add missing exports for FileBox and Strong #168

Merged
merged 1 commit into from Aug 18, 2021
Merged

Add missing exports for FileBox and Strong #168

merged 1 commit into from Aug 18, 2021

Conversation

vubogovich
Copy link
Contributor

@vubogovich vubogovich commented Aug 17, 2021

What

Add missing exports for FileBox and Strong.
Also remove the file with duplicated and unused exports. Looks like it's a left over from #43.

Why

Having two lists of component exports led to missing entries in the entry file.
Alternative: keep SSOT for component exports in components/index.js and export * from there in the entry file.

Who is affected

Those library consumers who imported components directly from components folder - hopefully nobody.


This change is Reviewable

Also remove the file with duplicated and unused exports.
@vubogovich vubogovich requested a review from a team as a code owner August 17, 2021 23:36
@changeset-bot
Copy link

changeset-bot bot commented Aug 17, 2021

🦋 Changeset detected

Latest commit: 8e5b1e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@qualifyze/design-system Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Aug 17, 2021

✔️ Deploy Preview for qualifyze-storybook ready!

🔨 Explore the source changes: 8e5b1e9

🔍 Inspect the deploy log: https://app.netlify.com/sites/qualifyze-storybook/deploys/611c47f54b53e300072190ae

😎 Browse the preview: https://deploy-preview-168--qualifyze-storybook.netlify.app

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8e5b1e9:

Sandbox Source
qf-design-system Configuration

@vubogovich vubogovich self-assigned this Aug 18, 2021
Copy link
Contributor

@sirJiggles sirJiggles left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @vubogovich)

a discussion (no related file):
If everything is still working then I am really happy sir, it did seem strange to have duplicate export files for this package. Just assumed it was needed in the past, my bad.

Just one question though, how come these things where not exported in the past? you think they where just forgotten or do you think they are so atomic that they where intentionally designed not to be exported? Kinda link internal workings of the DS components or something

I will mark it as discussing as I would not want to block you on my ignorance 💃


Copy link
Contributor Author

@vubogovich vubogovich left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @vubogovich)

a discussion (no related file):

Previously, sirJiggles (Gareth Fuller) wrote…

If everything is still working then I am really happy sir, it did seem strange to have duplicate export files for this package. Just assumed it was needed in the past, my bad.

Just one question though, how come these things where not exported in the past? you think they where just forgotten or do you think they are so atomic that they where intentionally designed not to be exported? Kinda link internal workings of the DS components or something

I will mark it as discussing as I would not want to block you on my ignorance 💃

Looks like the second file with exports appeared in the early stage of the repo when it was extracted from the monorepo.
In monorepo SSOT for exports was the root index.js. Then in new repo before #43 SSOT shifted to components/index.js and the root index.js export * from there. And during #43 we got two files with explicit exports.
I guess later someone added new exports to components/index.js and not to the root index.js. That's why we should not have two lists.
Checked with @gregoralbrecht that it's ok to export FileBox and Strong.


@vubogovich vubogovich merged commit ff01fcf into main Aug 18, 2021
@vubogovich vubogovich deleted the ssot-exports branch August 18, 2021 09:02
@github-actions github-actions bot mentioned this pull request Aug 18, 2021
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.

None yet

2 participants