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

fix(plugin-auto-nav-sidebar): Exclude config.route.exclude files from auto-generated sidebar #899

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Tyneor
Copy link

@Tyneor Tyneor commented Apr 2, 2024

Summary

When no _meta.json is found, instead of going through all children, use globby to exclude files defined in config.route.exclude.

Related Issue

#848 [Bug]: Excluded routes still appear in the no-config sidebars

Checklist

  • I have added changeset via pnpm run change.
  • I have updated the documentation.
  • I have added tests to cover my changes.

Tyneor added 3 commits April 2, 2024 16:29
…ion, fixed globby import, fixed empty folder appearing in sidebars, fixed changeset
@CLAassistant
Copy link

CLAassistant commented Apr 2, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

netlify bot commented Apr 2, 2024

Deploy Preview for aquamarine-blini-95325f ready!

Name Link
🔨 Latest commit ed69744
🔍 Latest deploy log https://app.netlify.com/sites/aquamarine-blini-95325f/deploys/661eb0725aa1d6000849f863
😎 Deploy Preview https://deploy-preview-899--aquamarine-blini-95325f.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 93 (no change from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

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

@Tyneor Tyneor changed the title Fix/848 Exclude config.route.exclude files from auto-generated sidebar fix(plugin-auto-nav-sidebar): Exclude config.route.exclude files from auto-generated sidebar Apr 2, 2024
sanyuan0704
sanyuan0704 previously approved these changes Apr 8, 2024
@@ -32,7 +34,15 @@ export async function scanSideMeta(
sideMeta = (await fs.readJSON(metaFile, 'utf8')) as SideMeta;
} catch (e) {
// If the `_meta.json` file doesn't exist, we will generate the sidebar config from the directory structure.
const subItems = await fs.readdir(workDir);

const includedFiles = globby.sync('**', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here globby.sync will traverse all the files and subdirectories under the directory but we only want to traverse the first level. You can change the params as follows:

globby.sync('*', {
  cwd: workDir,
  ignore: excludedFiles ?? [],
  expandDirectories: false,
  onlyFiles: false,
});

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
Author

Choose a reason for hiding this comment

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

Hello @sanyuan0704

This doesn't work for the use case assets/**/* I used in my original issue. With assets/**/* it is necessary to match the files under the directory.

I will still add expandDirectories: false + my second method because I think users excluding assets/**/* will be fine excluding assets/**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants