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

Better ignores implementation / documentation #2017

Closed
davestewart opened this issue Apr 17, 2023 · 5 comments · Fixed by #2022
Closed

Better ignores implementation / documentation #2017

davestewart opened this issue Apr 17, 2023 · 5 comments · Fixed by #2022
Assignees
Labels
enhancement New feature or request

Comments

@davestewart
Copy link
Contributor

davestewart commented Apr 17, 2023

Is your feature request related to a problem?

Overview

The current ignores implementation has a few issues:

  • options are converted to RegExps which only work as folder / file name prefixes
  • this a) is not made clear in the docs and b) could be avoided
  • content watching / refresh does not respect ignored files

Implementation

Looking at the ignore code, it means ignore options can only ever be prefixes:

export const contentIgnores: Array<RegExp> = contentConfig.ignores.map((p: any) =>
  typeof p === 'string' ? new RegExp(`^${p}\|:${p}`) : p

This makes sense in the terms of the supplied defaults:

ignores: ['\\.', '-'],

But is limiting in terms of more flexible ignoring (unless you get quite creative with the input strings).

Additionally: the implementation suggests that RegExps might be able to be passed, but they seem to get nuked by being squeezed through getRuntimeConfig().

Docs

The docs actually don't mention the prefixing, they suggest you can ignore words:

This should be clarified.

File watching

It looks like the ignores config doesn't have any effect on the storage instance at the bottom of src/module.ts.

Saving ignored files still triggers refreshNuxtData() via a sockets call.

Describe the solution you'd like

To support the documented ignores format, change the default to:

const ignores: string[] = [
  '^[.-]|:[.-]'
]

And simplify the contentIgnores map to:

const contentIgnores = contentConfig.ignores.map(p => new RegExp(p))

Although, if prefixing is a specific design decision, then document it and provide a couple of examples:

[
  '.+?\\.txt$' // converts to /:.+?\.txt$/, matching text files only
]

To prevent triggering a refresh when changing ignored files, in the module watch use the same algorithm to prevent a sockets broadcast:

      await nitro.storage.watch(async (event: WatchEvent, key: string) => {
        // Ignore events that are not related to content
        if (isIgnored(key)) {
          return
        }
        ...
      })

Finally, as the ignore functionality will be used three times at this point, consolidate into a reusable function:

Describe alternatives you've considered

I've been poring through the ignores code in the IDE and debugger, and pretty sure this is the only option.

Additional context

Trying to optimise the updating code in Nuxt Content Assets:

Pretty sure I know enough about Nuxt Content's source now to do a PR.

@farnabaz
Copy link
Member

Hey @davestewart,
Thanks for explanation, Feel free to open a PR 👍
Looking forward to seeing your improvement 🙂

@davestewart
Copy link
Contributor Author

davestewart commented Apr 17, 2023

Great!

Any preference regarding the prefix strategy?

I know Content heavily uses unstorage, hence the : delimiters.

Ultimately, this means any ignore pattern targets single folder / files names only.

Would you prefer to document or expand this functionality?

@farnabaz
Copy link
Member

The prefix ignoring is adopted from Nuxt itself (Nuxt ignores files with - prefix). So to be consistent, we should keep ignoring these prefixes. But it will be nice to support different kind of ignore patterns.

What you mean by expanding it? I didn't get your idea here.

@davestewart
Copy link
Contributor Author

davestewart commented Apr 17, 2023

Ah... sorry!

I meant the : delimiter (not prefix!):

new RegExp(`^${p}|:${p}`)

This will match the start or any segment (i.e. :<token>) of an unstorage key:

sources:content:path:to:file.md
sources:content:file.md
file.md

What I meant was, do you want to:

  • maintain the transformation of any user-supplied ignore pattern (so it is anchored to unstorage's : delimiters)
  • allow any pattern to be passed (and modify the defaults so they still match . and -)

Copy link
Member

I thinks the second one is better approach. Users can design their pattern whatever they want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants