Skip to content

docs(RFC): Update File Organization Convention for v9 RFC to address other issue#24543

Merged
TristanWatanabe merged 6 commits intomicrosoft:masterfrom
TristanWatanabe:update-rfc
Sep 29, 2022
Merged

docs(RFC): Update File Organization Convention for v9 RFC to address other issue#24543
TristanWatanabe merged 6 commits intomicrosoft:masterfrom
TristanWatanabe:update-rfc

Conversation

@TristanWatanabe
Copy link
Member

@TristanWatanabe TristanWatanabe commented Aug 26, 2022

Changes

  • Updates File Organization Convention for v9 Packages RFC to reflect removal of problematic common/ folder and adds proposed replacement for it. Also provides a more detailed outline of how the components/, utils/ and new testing/ subfolders should be structured.
  • Also adds an update to remove the e2e/ subfolder in favor of collocating cypress tests with the implementation and unit/jest files.

PREVIEW

Follow-up to #24332 (comment)
Part of #24129

@github-actions github-actions bot added the Type: RFC Request for Feedback label Aug 26, 2022
@github-actions github-actions bot added this to the July Project Cycle Q3 2022 milestone Aug 26, 2022
@TristanWatanabe TristanWatanabe changed the title Update package structure with details docs(RFC): Update File Organization Convention for v9 RFC to address other issue Aug 26, 2022
@fabricteam
Copy link
Collaborator

fabricteam commented Aug 26, 2022

📊 Bundle size report

🤖 This report was generated against adea89e6cc51161fbb544f465e298f60046fd55c

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 26, 2022

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 8378977:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@TristanWatanabe TristanWatanabe marked this pull request as ready for review August 26, 2022 16:14
@TristanWatanabe TristanWatanabe requested review from a team August 26, 2022 16:14
@size-auditor
Copy link

size-auditor bot commented Aug 26, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: adea89e6cc51161fbb544f465e298f60046fd55c (build)

|- {ComponentName}.tsx
|- {ComponentName}.types.ts
|- {ComponentName}.test.tsx
|- {ComponentName}.e2e.tsx
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: i have mixed feelings about this. while I understand the reasoning because we have all the context, it might feel missleading to OSS contributors what e2e actually means.

Nx uses cy suffix, which explicitly express the intent {ComponentName}.cy.tsx (cypress component testing)

One downside of this is if we gonna switch to something else in the future, we would need to change the suffix ( lets say we switch to playwright ).

WDYT?

Copy link
Member Author

@TristanWatanabe TristanWatanabe Sep 27, 2022

Choose a reason for hiding this comment

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

I'm in favor of being more explicit and using .cy. - if we have to switch to another solution in the future, then it should be updated accordingly.

|- index.ts
|- shared-component-types.types.ts
|- some-function-or-hook.ts
|- testing/
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe utils-testing ? to mirror the utils approach ?

We're already following this convention when it comes to e2e so most of the work will be extracting stories out of the `src` folder and moving those to the root of the package. The asset files will also need to be moved to the appropriate `assets` subfolder. And finally, the `.npmignore` file will then be updated to ignore any asset files and files living within the documentation folder.
We will be extracting stories out of the `src` folder and moving those to the root of the package within a `stories/` subfolder. The asset files will also need to be moved to the appropriate `assets` subfolder. The `.npmignore` file will then be updated to ignore any asset files and files living within the documentation folder.

Also, the old `common/` folder which caused confusion and unexpected linting errors will now be replaced with a more robust `testing/` subfolder within the `src` folder to house the conformance testing factory and any mock or utility testing files to be used in multiple tests within a package.
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌

Copy link
Contributor

@Hotell Hotell left a comment

Choose a reason for hiding this comment

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

some thoughts and suggestions. lets keep the convo going. otherwise LGTM

@TristanWatanabe TristanWatanabe merged commit c37c871 into microsoft:master Sep 29, 2022
@TristanWatanabe TristanWatanabe deleted the update-rfc branch September 29, 2022 14:53
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: RFC Request for Feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants