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

feat(msw): create an index file exporting msw for split tags mode #1803

Merged

Conversation

AllieJonsson
Copy link
Contributor

@AllieJonsson AllieJonsson commented Jan 9, 2025

Status

READY/WIP

Description

Fixes #512
Is index.msw.ts a good name? Or should it be something like mocks.ts?

Steps to Test or Reproduce

  1. yarn generate:mock
  2. check generated/mock/petstore-tags-split and there is a index.msw.ts file which exports * from all .msw.ts files

@anymaniax
Copy link
Collaborator

anymaniax commented Jan 9, 2025

it's not a fix for me but more a feature no? And for the file name I would index.mocks.ts instead to follow the logic of the packages no? @melloware @soartec-lab what do you think?

@melloware
Copy link
Collaborator

Agreed i think index.mock.ts would be good.

@soartec-lab
Copy link
Member

I completely agree with index.msw.ts 👍
This is a great enhancement!

Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

If we don't support everything at once and continue to use it as before, this aggregated index will be unnecessary and just noise. How about making it optional?

@AllieJonsson
Copy link
Contributor Author

If we don't support everything at once and continue to use it as before, this aggregated index will be unnecessary and just noise. How about making it optional?

I added a check for output.indexFiles, so you can set indexFiles: false in config to not generate this. Or did you have something else in mind?

@anymaniax
Copy link
Collaborator

indexFiles also handle the index.ts too so I would add another property like indexMockFiles (maybe there is something better). Like that we can have index.ts and index.(mocks|msw).ts handled separatly

@anymaniax
Copy link
Collaborator

@AllieJonsson also thanks for all you are doing!

@AllieJonsson
Copy link
Contributor Author

indexFiles also handle the index.ts too so I would add another property like indexMockFiles (maybe there is something better). Like that we can have index.ts and index.(mocks|msw).ts handled separatly

True, I just thought that if you want to have index files for non-mock files, you probably want it for mock files as well. But maybe separating and allowing just having one of them is better. Should it be enabled or disabled by default? indexFiles is true by default

@anymaniax
Copy link
Collaborator

good point @soartec-lab @melloware I am ok with it either way so if you have thoughts

@AllieJonsson
Copy link
Contributor Author

This is also really only valid for tags-split mode, right? So adding a whole new setting that only applies to this mode is maybe a bit overkill

@AllieJonsson
Copy link
Contributor Author

Or maybe this should be added to mode tags as well?

@melloware
Copy link
Collaborator

yeah we have too many settings already @AllieJonsson 😄

@soartec-lab
Copy link
Member

@AllieJonsson

It's exactly as @anymaniax wrote.
When using tags-split, there are cases where the schema needs indexing and mocking is not necessary, but we don't want to end up with exclusive control.

Also, reusing one option for different purposes will complicate things and confuse users. I agree that there are already too many configuration items, but I didn't think it was overkill to create one option for a new use case.

Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

Another comment I made was about the file extension. The only file type available is msw, so I would like to align them.

packages/core/src/writers/split-tags-mode.ts Outdated Show resolved Hide resolved
@soartec-lab soartec-lab added enhancement New feature or request msw MSW related issues labels Jan 10, 2025
@soartec-lab soartec-lab added this to the 7.4.2 milestone Jan 10, 2025
@AllieJonsson
Copy link
Contributor Author

Wait, this setting should be in output.mock.indexMockFiles instead of output.indexMockFiles...

Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

@AllieJonsson
I made one comment to the docs. The rest are fine. Thanks for the great enhance!

docs/src/pages/reference/configuration/output.md Outdated Show resolved Hide resolved
Copy link
Member

@soartec-lab soartec-lab left a comment

Choose a reason for hiding this comment

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

Great!

@soartec-lab soartec-lab merged commit 6938ec7 into orval-labs:master Jan 13, 2025
2 checks passed
@AllieJonsson AllieJonsson deleted the feat/split-tags-mock-index branch January 14, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request msw MSW related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mocks: Is there a way to have a single export for mocks in tags-split mode?
4 participants