feat: more flexible ignores configuration #2022

davestewart · 2023-04-17T16:10:02Z

🔗 Linked issue

❓ Type of change

📖 Documentation (updates to the documentation or readme)
🐞 Bug fix (a non-breaking change that fixes an issue)
👌 Enhancement (improving an existing functionality like performance)
✨ New feature (a non-breaking change that adds functionality)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This PR enhances Nuxt Content's ignores implementation.

It has three main aims:

make the options more flexible
add a single factory function to test ignore patterns
ensure ignored files don't trigger an application refresh

Options

Right now, the ignores implementation transforms user-supplied options so that they work with atoms of the unstorage path syntax, by converting the string to a RegExp and prefixing it with a : delimiter (which equates to /).

Thus, 'hidden' will only ever match /hidden and not /my-hidden-folder. In order for a user to get around this they would need some additional RegExp acrobatics, i.e. '.+hidden'. Ignoring file extensions would require '.+\\.ext' rather than the more idiomatic '\\.ext$'.

This change gives the user the flexibility to decide how to ignore content, whist respecting the current defaults to ignore .dot and -dash files.

Predicate

The new makeIgnores() factory function returns a predicate which eliminates the duplicate code from the rest of the module.

Watch

Currently the module's watch functionality doesn't respect ignored files, and triggers an application refresh even when ignored files are saved.

The update uses the predicate function to ignore ignored file updates and not refresh the app.

Testing

I'm not 100% sure how to test this functionality using a test framework, but I have updated the playground/document-driven to test it out in the browser.

Resolves #2017

📝 Checklist

I have linked an issue or discussion.
I have updated the documentation accordingly.

codesandbox · 2023-04-17T16:10:07Z

Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

nuxt-studio · 2023-04-17T16:10:08Z

✅ Live Preview ready!

Name	Edit	Preview	Latest Commit
content	Edit on Studio ↗︎	View Live Preview	`1f3fc27`

nuxt-studio · 2023-04-17T16:10:11Z

✅ Live Preview ready!

Name	Edit	Preview	Latest Commit
content	Edit on Studio ↗︎	View Live Preview	`912c495`

netlify · 2023-04-17T16:10:12Z

✅ Deploy Preview for nuxt-content ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`1f3fc27`
🔍 Latest deploy log	https://app.netlify.com/sites/nuxt-content/deploys/643fc4223c90580008160ce6
😎 Deploy Preview	https://deploy-preview-2022--nuxt-content.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

davestewart · 2023-04-17T19:00:02Z

I just generated the document driven playground with the some configured ignore rules, and it doesn't generate, so I guess it works!

farnabaz · 2023-04-18T10:31:51Z

Thanks for the PR,
It looks good to me 👍

The only concern I have is about the breaking change we are creating with this PR.
Do you see any possible workaround to prevent the breaking change?
Or maybe detect it and warn users.

davestewart · 2023-04-18T11:15:42Z

That was my only concern too.

Could potentially:

create a new config option ignore for the more flexible functionality
deprecate old option
handle both options in the one factory function until next major version

Feels a bit confusing though.

Or:

create an experimental config option ignores for the more flexible functionality
handle both options in the one factory function until it is decided to include it

I've seen it in Nuxt, haven't really looked at it. How do they choose / document successful experiments to the core?

Or:

restore the : delimited functionality
update documentation examples to include the '\\.*' workaround

Feel safer.

My hunch is that the existing ignores config is too esoteric for anyone to have figured it out otherwise (but that offers no guarantees!).

Thinking about it; the new ignores options widen the scope for ignored content, so the worst that could happen is less content is published, rather than something sensitive being made available:

hidden - used to match :hidden-folder now matches :my-hidden-folder
workaround is to use :hidden

Regarding any warnings, I don't have any experience in this. Would you use something like a postinstall hook to check the config so the warning is only shown once? Or just in the module's setup function? And it would be shown every time?

farnabaz · 2023-04-18T11:40:37Z

create an experimental config option ignores for the more flexible functionality

handle both options in the one factory function until it is decided to include it

I like this approach.

We can enable this new feature under the flag.
In module check if the user registered any ignores pattern and warn user about the breaking change in v2.7

davestewart · 2023-04-18T11:44:43Z

Aha! It exists!

https://github.com/nuxt/content/blob/main/src/module.ts#L281-L284

OK, I'll get that written now 👍

Is there any existing format for warnings? Just console.warn() or an existing utility function?

farnabaz · 2023-04-18T11:48:35Z

We can change docs a bit to introduce this experimental flag (the ones you've added in the PR).
Then we can use consola.warn to briefly note about behaviour change and link to docs.

WDYT?

davestewart · 2023-04-18T11:58:33Z

OK, copying the example here:

https://github.com/nuxt/nuxt/blob/886350a27c6b733ae07dfd5f6b8457214aa83fe6/packages/nuxi/src/commands/build.ts#L52

consola.warn('The `ignores` config is being made more flexible in version 2.7. See the docs for more information: `https://content.nuxtjs.org/api/configuration#ignores`')

davestewart · 2023-04-18T14:36:01Z

OK! All done.

Sorry it took me so long, but here are the main changes:

Format

I made a small modification so the new format now supports / directly in the pattern!

This means users won't need to use : in place of /

`makeIgnores()`

This now takes the runtime config directly, and ignores and experimental.ignores patterns are grabbed.

If the user passes experimental options, the new algorithm is used in place of the old one.

`defaults`

I've moved the .dot and .dash defaults to the factory function.

This is because:

the normal and experimental prefixes are subtly different
the normal and experimental options are declared in two different places
the normal and experimental options are JSDoc'ed in two different places

All the jumping about, just for config's sake, could lead to errors later.

Additionally, this makes the warning logic less magic; any supplied ignore options (vs > "magic" 2) will be caught.

Playground

I moved the ignore examples to /basic as it is a basic feature.

I also added { 'ignores: 'blah' } in /basic to trigger the warning

Docs

The docs now explain how the two versions differ, and how to use the new version.

I hope that's all clear and good to go!

farnabaz · 2023-04-18T15:15:30Z

I think it's better to just have an experimental flag, and use the same ignores option to define new pattern. This way upgrading to v2.7 will be easier, just removing the flag.

export default defineNuxtConfig({
  content: {
     experimental: {
       advancedIgnoresPattern: true
     },
     ignores: [
       'hidden',
       '/hidden/',
       // ...
     ]
  }
})

WDYT?

…Pattern

davestewart · 2023-04-18T15:34:55Z

OK, done!

farnabaz

👍

src/module.ts

farnabaz · 2023-04-18T15:41:56Z

@davestewart Looks great.

Do you mind fixing CI?

Co-authored-by: Farnabaz <farnabaz@gmail.com>

davestewart · 2023-04-18T16:00:14Z

Re. release, I haven't updated the package version or anything.

How are releases normally coordinated?

davestewart · 2023-04-19T08:29:11Z

BTW, writing some tests for this, this morning.

farnabaz · 2023-04-19T09:34:16Z

I'm planning to release v2.6 today

Also, before that, once we merge this PR you can test it on edge-channel

davestewart · 2023-04-19T10:29:04Z

Question, for future reference...

I presume PRs are squashed, so as per the Conventional Commits docs – do I need to use the task: description format in the feature branch if you'll just squash everything with a feat: description anyway?

Do all my contributors need to use the Conventional Commits specification?

No! If you use a squash based workflow on Git lead maintainers can clean up the commit messages as they’re merged—adding no workload to casual committers. A common workflow for this is to have your git system automatically squash commits from a pull request and present a form for the lead maintainer to enter the proper git commit message for the merge.

It's a bit of a pain choosing which prefix to use with so many small updates...

farnabaz · 2023-04-19T10:52:49Z

Not sure I'm getting your point here. But For me this is an improvement and new feature so I'd use feat for it.
As for the branch it does not matter. As you said, PR will be squashed so the only thing matters is PR title

davestewart · 2023-04-19T10:54:52Z

Yeah I meant for those in the branch.

OK, good to know, thanks.

farnabaz

👍

farnabaz · 2023-04-19T11:07:19Z

Let's wait for CI to finish and you can test in edge channel

davestewart · 2023-04-19T11:31:03Z

FWIW I get a 503 at my end with the edge channel:

file:///Volumes/Data/Work/OpenSource/JavaScript/NuxtContentAssets/nuxt-content-assets/demo/.nuxt/dev/index.mjs:56
import { consola } from 'file:///Volumes/Data/Work/OpenSource/JavaScript/NuxtContentAssets/nuxt-content-assets/demo/node_modules/unenv/runtime/npm/consola.mjs';
         ^^^^^^^
SyntaxError: The requested module 'file:///Volumes/Data/Work/OpenSource/JavaScript/NuxtContentAssets/nuxt-content-assets/demo/node_modules/unenv/runtime/npm/consola.mjs' does not provide an export named 'consola'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:533:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

Have hacked node modules so I can test the feature.

farnabaz · 2023-04-19T11:35:36Z

Which version of Nuxt are you using?
Did you try removing lock file and reinstalling?

davestewart · 2023-04-19T11:46:15Z

OK, did that and still get it:

[12:43:58 PM] ℹ [content-assets] Websocket listening on "ws://localhost:4001/"
[12:44:02 PM] ℹ Vite client warmed up in 2666ms
[nitro 12:44:02 PM] ✔ Nitro built in 1323 ms
[12:44:12 PM] [nuxt] [request error] [unhandled] [500] Named export 'consola' not found. The requested module 'consola' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'consola';
const { consola } = pkg;

  at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)  
  at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)  
  at async Promise.all (index 0)  
  at async ESMLoader.import (node:internal/modules/esm/loader:533:24)  
  at async ./demo/.nuxt/dev/index.mjs:4138:24  
  at async ./demo/.nuxt/dev/index.mjs:4217:64  
  at async ./demo/.nuxt/dev/index.mjs:733:22  
  at async Object.handler (./node_modules/h3/dist/index.mjs:1247:19)  
  at async Server.toNodeHandle (./node_modules/h3/dist/index.mjs:1322:7)
[12:44:12 PM] [nuxt] [request error] [unhandled] [500] Named export 'consola' not found. The requested module 'consola' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'consola';
const { consola } = pkg;

  at ModuleJob._instantiate (node:internal/modules/esm/module_job:123:21)  
  at async ModuleJob.run (node:internal/modules/esm/module_job:189:5)  
  at async Promise.all (index 0)  
  at async ESMLoader.import (node:internal/modules/esm/loader:533:24)  
  at async ./demo/.nuxt/dev/index.mjs:4138:24  
  at async ./demo/.nuxt/dev/index.mjs:4217:64  
  at async ./demo/.nuxt/dev/index.mjs:733:22  
  at async Object.handler (./node_modules/h3/dist/index.mjs:1247:19)  
  at async Server.toNodeHandle (./node_modules/h3/dist/index.mjs:1322:7)

- Operating System: `Darwin`
- Node Version:     `v16.17.1`
- Nuxt Version:     `3.4.1`
- Nitro Version:    `2.3.3`
- Package Manager:  `npm@8.15.0`
- Builder:          `vite`
- User Config:      `app`, `modules`, `content`, `contentAssets`
- Runtime Modules:  `../src/module`, `@nuxt/content-edge`
- Build Modules:    `-`

davestewart · 2023-04-19T11:54:08Z

Looks like line 2 maybe?

https://github.com/nuxt/content/blame/12d5a21e62b380889bc0767bb08d7c9b18ff26bd/src/runtime/transformers/shiki/highlighter.ts#L2

farnabaz · 2023-04-19T12:02:05Z

This is strange, named export is suggested why to import consola. I just tested edge channel in new project, and couldn't reproduce it.

Do you have consola as direct dependency, or maybe as sub-dep of another package?

Please check which version of consola you are using npm why consola

davestewart · 2023-04-19T12:17:07Z

OK, it looks like I had a reference to @nuxt/content in my peer dependencies and that was installing the old version.

Have just nuked everything and rerun and it compiles!

Bloody computers 😆 (/humans)

davestewart · 2023-04-19T12:20:04Z

Also, pleased to report that the updates mean I can get the next couple of optimisations for Nuxt Content Assets out!

Thanks so much for working through this PR with me. Really appreciated!

davestewart added 3 commits April 17, 2023 16:45

feat: factory function to create a unified ignores predicate

64ea229

feat: add ignore examples to playground

7dbaf4e

docs: update ignores documentation

912c495

davestewart and others added 2 commits April 17, 2023 17:14

Merge branch 'main' into feat/improve-ignores

7419c5f

feat: add index to playground

29e94d3

davestewart mentioned this pull request Apr 18, 2023

Update content on image update davestewart/nuxt-content-assets#31

Merged

Merge branch 'main' into feat/improve-ignores

b5f84b5

davestewart and others added 4 commits April 18, 2023 14:45

Merge branch 'main' into feat/improve-ignores

cda7da6

feat: modify makeIgnores() helper to support old and new style patterns

1c863b2

fix: move playground examples to basic

54e0395

feat: update docs

98cf14b

fix: simplify ignores factory

7c58d0f

fix: refactor experimental.ignores to be experimental.advancedIgnores…

e04a3aa

…Pattern

farnabaz approved these changes Apr 18, 2023

View reviewed changes

src/module.ts Outdated Show resolved Hide resolved

src/module.ts Outdated Show resolved Hide resolved

fix: correct flag

4217d6b

Co-authored-by: Farnabaz <farnabaz@gmail.com>

fix: linting

4771342

fix: runtimeConfig defaults

ce745c0

davestewart mentioned this pull request Apr 18, 2023

Update related documents when images change davestewart/nuxt-content-assets#30

Closed

davestewart changed the title ~~Feat: improve ignores~~ feat: improve ignores Apr 18, 2023

davestewart added 2 commits April 19, 2023 11:35

refactor: improve makeIgnores()

eaea7c7

test: add ignores tests

1f3fc27

davestewart changed the title ~~feat: improve ignores~~ feat: more flexible ignores configuration Apr 19, 2023

farnabaz approved these changes Apr 19, 2023

View reviewed changes

farnabaz merged commit 12d5a21 into nuxt:main Apr 19, 2023
7 checks passed

farnabaz mentioned this pull request Apr 19, 2023

v2.6 #2031

Merged

davestewart deleted the feat/improve-ignores branch April 20, 2023 10:14

davestewart mentioned this pull request Apr 20, 2023

feat(config): update ignores to advanced pattern #2035

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: more flexible ignores configuration #2022

feat: more flexible ignores configuration #2022

davestewart commented Apr 17, 2023 •

edited

codesandbox bot commented Apr 17, 2023

nuxt-studio bot commented Apr 17, 2023 •

edited

nuxt-studio bot commented Apr 17, 2023

netlify bot commented Apr 17, 2023 •

edited

davestewart commented Apr 17, 2023

farnabaz commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023 •

edited

farnabaz commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023

farnabaz commented Apr 18, 2023

davestewart commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023 •

edited

farnabaz commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023

farnabaz left a comment

farnabaz commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023 •

edited

davestewart commented Apr 19, 2023

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023 •

edited

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023

farnabaz left a comment

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023

davestewart commented Apr 19, 2023 •

edited

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023 •

edited

davestewart commented Apr 19, 2023

feat: more flexible ignores configuration #2022

feat: more flexible ignores configuration #2022

Conversation

davestewart commented Apr 17, 2023 • edited

🔗 Linked issue

❓ Type of change

📚 Description

Options

Predicate

Watch

Testing

📝 Checklist

codesandbox bot commented Apr 17, 2023

nuxt-studio bot commented Apr 17, 2023 • edited

✅ Live Preview ready!

nuxt-studio bot commented Apr 17, 2023

✅ Live Preview ready!

netlify bot commented Apr 17, 2023 • edited

✅ Deploy Preview for nuxt-content ready!

davestewart commented Apr 17, 2023

farnabaz commented Apr 18, 2023 • edited

davestewart commented Apr 18, 2023 • edited

farnabaz commented Apr 18, 2023 • edited

davestewart commented Apr 18, 2023

farnabaz commented Apr 18, 2023

davestewart commented Apr 18, 2023 • edited

davestewart commented Apr 18, 2023 • edited

Format

makeIgnores()

defaults

Playground

Docs

farnabaz commented Apr 18, 2023 • edited

davestewart commented Apr 18, 2023

farnabaz left a comment

Choose a reason for hiding this comment

farnabaz commented Apr 18, 2023 • edited

davestewart commented Apr 18, 2023 • edited

davestewart commented Apr 19, 2023

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023 • edited

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023

farnabaz left a comment

Choose a reason for hiding this comment

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023

davestewart commented Apr 19, 2023 • edited

farnabaz commented Apr 19, 2023

davestewart commented Apr 19, 2023 • edited

davestewart commented Apr 19, 2023

davestewart commented Apr 17, 2023 •

edited

nuxt-studio bot commented Apr 17, 2023 •

edited

netlify bot commented Apr 17, 2023 •

edited

farnabaz commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023 •

edited

farnabaz commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023 •

edited

`makeIgnores()`

`defaults`

farnabaz commented Apr 18, 2023 •

edited

farnabaz commented Apr 18, 2023 •

edited

davestewart commented Apr 18, 2023 •

edited

davestewart commented Apr 19, 2023 •

edited

davestewart commented Apr 19, 2023 •

edited

davestewart commented Apr 19, 2023 •

edited