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

Make template lint use node glob to ensure consistent lint results across OS #1988

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

Elyseum
Copy link
Contributor

@Elyseum Elyseum commented Nov 29, 2024

Description / Motivation

This PR updates the npm lint command to use the node glob pattern (instead of the OS glob) for scaffolded projects. This makes linting results more consistent across different OS (e.g. windows development machine vs linux build server).

See https://stackoverflow.com/questions/54165756/eslint-glob-is-not-considering-all-directories-recursively

This node glob was introduced silently in the JSS development scripts (see 786913d), but it never made its way to the scaffolding templates.

Testing Details

  • Unit Test Added
  • Manual Test/Other (Please elaborate)

I experienced this with an lint issue in my (customized) packages\create-sitecore-jss\src\templates\nextjs-sxa\src\Layout.tsx file. The lint error was detected on Windows (**/*.tsx matches direct children), but not on Ubuntu linux (**/*.tsx does NOT match direct children).
After forcing to node glob pattern, the lint issue was detected on both Windows AND (Ubuntu) linux.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@Elyseum Elyseum changed the title Feature/template node glob 2 Make template lint use node glob to ensure consistent lint results across OS Nov 29, 2024
@Elyseum
Copy link
Contributor Author

Elyseum commented Nov 29, 2024

@art-alexeyenko I've added the escaping to vue template as well. I uses vue-cli-service lint instead of eslint. That's why I did not spot it at first. I don't know the tool, but it looks like it uses eslint behind the scenes (as the package is called @vue/cli-plugin-eslint), so we can assume it supports the glob as well.

@art-alexeyenko
Copy link
Contributor

@Elyseum appreciate it!
I've put the task to review/test this into our backlog. I'll try to make time for me and the team to look at it earlier, more carefully now. It may take a bit, but we'll update you as soon as there's movement with this PR.

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

Successfully merging this pull request may close these issues.

2 participants