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: Add and export isFilenameValid function #951

Merged
merged 3 commits into from
May 21, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented May 16, 2024

No description provided.

@susnux susnux requested review from artonge, skjnldsv and Pytal May 16, 2024 16:14
@susnux susnux added enhancement New feature or request 3. to review labels May 16, 2024
@susnux

This comment was marked as resolved.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the feat/export-filename-unti branch from b8edb41 to 8a06891 Compare May 16, 2024 18:05
@susnux susnux requested a review from ShGKme May 16, 2024 18:06
Copy link

codecov bot commented May 16, 2024

Bundle Report

Changes will increase total bundle size by 1.04kB ⬆️

Bundle name Size Change
@nextcloud/files-esm 105.32kB 507 bytes ⬆️
@nextcloud/files-cjs 106.27kB 531 bytes ⬆️

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
})

it('works for valid filenames', async () => {
const { isFilenameValid } = await import('../lib/index')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why no global import? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to reset the module for every test to reset the cached forbidden characters

Copy link

Choose a reason for hiding this comment

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

Doesn't Vitest have reset module option?

Copy link

Choose a reason for hiding this comment

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

With vi.resetModules()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't Vitest have reset module option?

Yes see rest of the file, but it can only reset the node cache. So top level imports do not work. It only resets the module cache -> next import will be freshly evaluated.

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 we always on edge cases 🙈 😭

@susnux susnux requested a review from skjnldsv May 16, 2024 19:55
@skjnldsv skjnldsv merged commit 7c53525 into main May 21, 2024
16 checks passed
@skjnldsv skjnldsv deleted the feat/export-filename-unti branch May 21, 2024 07:08
@skjnldsv skjnldsv mentioned this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants