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

Enforce Pascal casing of component name in template in the docs package #4004

Merged
merged 80 commits into from
Nov 22, 2023

Conversation

xiongmao86
Copy link
Contributor

Make sure docs examples using Pascal case in the example of docs package.
Fix #3989.

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 not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

@xiongmao86
Copy link
Contributor Author

I have met the following error when I yarn workspace docs lint and also yarn lint under the docs directory.

Here is the error message:

yarn run v1.22.19
$ eslint --ext .js,.ts,.vue .

Oops! Something went wrong! :(

ESLint: 7.32.0

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Failed to load plugin '@typescript-eslint' declared in '.eslintrc.js » @nuxt/eslint-config': Package subpath './lib/definition' is not defined by "exports" in C:\Users\xiongmao86\projects\vuestic-ui\node_modules\@nuxt\eslint-config\node_modules\eslint-scope\package.json
Referenced from: C:\Users\xiongmao86\projects\vuestic-ui\node_modules\@nuxt\eslint-config\index.js
    at new NodeError (node:internal/errors:372:5)
    at throwExportsNotFound (node:internal/modules/esm/resolve:472:9)
    at packageExportsResolve (node:internal/modules/esm/resolve:753:3)
    at resolveExports (node:internal/modules/cjs/loader:482:36)
    at Function.Module._findPath (node:internal/modules/cjs/loader:522:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:919:27)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (C:\Users\xiongmao86\projects\vuestic-ui\node_modules\v8-compile-cache\v8-compile-cache.js:159:20)
    at Object.<anonymous> (C:\Users\xiongmao86\projects\vuestic-ui\node_modules\@nuxt\eslint-config\node_modules\@typescript-eslint\utils\dist\ts-eslint-scope\Definition.js:4:22)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@m0ksem
Copy link
Collaborator

m0ksem commented Nov 13, 2023

I have met the following error when I yarn workspace docs lint and also yarn lint under the docs directory.

Maybe you need to run yarn install, works well for me.

@xiongmao86
Copy link
Contributor Author

Hi, @m0ksem, it is not working after I yarn install --force, and run yarn lint in packages/docs, it doesn't work. I have received the same output.

@m0ksem
Copy link
Collaborator

m0ksem commented Nov 13, 2023

@asvae, can you check if you have same error?

@asvae asvae self-requested a review November 14, 2023 21:31
@asvae
Copy link
Member

asvae commented Nov 14, 2023

Nah, it's all good for me:

➜  6-vuestic-ui git:(fix/docs-component-name-casing) yarn workspace docs lint --fix
yarn workspace v1.22.19
yarn run v1.22.19
$ eslint --ext .js,.ts,.vue . --fix

/Users/yauheniprakopchyk/2-Projects/7-epicmax/1-epicmax/6-vuestic-ui/packages/docs/components/ComponentPlayground.vue
  2:10  warning  'ComponentOptions' is defined but never used  @typescript-eslint/no-unused-vars

/Users/yauheniprakopchyk/2-Projects/7-epicmax/1-epicmax/6-vuestic-ui/packages/docs/composables/useComponentPlayground.ts
  65:28  warning  'defaultValue' is defined but never used  @typescript-eslint/no-unused-vars

/Users/yauheniprakopchyk/2-Projects/7-epicmax/1-epicmax/6-vuestic-ui/packages/docs/layouts/default.vue
  45:9  warning  'currentPresetName' is assigned a value but never used  @typescript-eslint/no-unused-vars

/Users/yauheniprakopchyk/2-Projects/7-epicmax/1-epicmax/6-vuestic-ui/packages/docs/modules/banner/module.ts
  23:9  warning  'options' is defined but never used  @typescript-eslint/no-unused-vars

/Users/yauheniprakopchyk/2-Projects/7-epicmax/1-epicmax/6-vuestic-ui/packages/docs/modules/page-config/blocks/alert/index.vue
  4:7  warning  'props' is assigned a value but never used  @typescript-eslint/no-unused-vars

/Users/yauheniprakopchyk/2-Projects/7-epicmax/1-epicmax/6-vuestic-ui/packages/docs/modules/page-config/blocks/api/index.vue
   90:15  warning  'name' is defined but never used     @typescript-eslint/no-unused-vars
  103:13  warning  'key' is defined but never used      @typescript-eslint/no-unused-vars
  136:15  warning  'prop' is defined but never used     @typescript-eslint/no-unused-vars
  145:82  warning  'comment' is defined but never used  @typescript-eslint/no-unused-vars

Maybe node version or something? I'm using nvm use in project root to set the version we use.

Also rm -rf node_modules, then yarn might work.

@asvae
Copy link
Member

asvae commented Nov 14, 2023

It also doesn't appear this PR is fixing the issue.

I checked the file I mentioned after lint and there is nothing in the log.

Users/yauheniprakopchyk/2-Projects/7-epicmax/1-epicmax/6-vuestic-ui/packages/docs/page-config/ui-elements/skeleton/examples/GroupWave.vue
   5:38  warning  'width' should be on a new line    vue/max-attributes-per-line
   5:51  warning  'height' should be on a new line   vue/max-attributes-per-line
   6:36  warning  'class' should be on a new line    vue/max-attributes-per-line
   6:49  warning  ':lines' should be on a new line   vue/max-attributes-per-line
   8:49  warning  'style' should be on a new line    vue/max-attributes-per-line
   9:34  warning  'variant' should be on a new line  vue/max-attributes-per-line
   9:52  warning  'inline' should be on a new line   vue/max-attributes-per-line
   9:59  warning  'width' should be on a new line    vue/max-attributes-per-line
   9:72  warning  'height' should be on a new line   vue/max-attributes-per-line
  10:39  warning  'inline' should be on a new line   vue/max-attributes-per-line
  10:46  warning  'width' should be on a new line    vue/max-attributes-per-line
  10:59  warning  'height' should be on a new line   vue/max-attributes-per-line

@asvae
Copy link
Member

asvae commented Nov 14, 2023

I'm thinking an greasy-easy fix for this could be a replacement regex from <va-whatever to <VaWhatever .

Linter might be tricky to arrange.

@xiongmao86
Copy link
Contributor Author

@asvae, do you mean change by regex replace manually? Does the lint rule I added matters in this case? Should I remove it?

@m0ksem
Copy link
Collaborator

m0ksem commented Nov 15, 2023

@asvae, do you mean change by regex replace manually? Does the lint rule I added matters in this case? Should I remove it?

I think eslint rule is a perfect solution. But we definitely need be able to run eslint on windows. I assume Package subpath './lib/definition' is working incorrectly in windows, so we'll need to upgrade this package I guess. Or make an issue in @typescript-eslint if upgrade do not help.

@xiongmao86 what's your node version? I'm wondering how vuestic-ui exports work in such case.

@xiongmao86
Copy link
Contributor Author

Hi, @m0ksem, my node version is v16.15.0. I try to bump eslint dependency to 8.53.0, and yarn lint in docs works again. Can I just bump eslint and continue with my work?

@xiongmao86 xiongmao86 force-pushed the fix/docs-component-name-casing branch from 69e8ea2 to 36df497 Compare November 18, 2023 12:17
@xiongmao86
Copy link
Contributor Author

This pr is ready for review.

@asvae
Copy link
Member

asvae commented Nov 22, 2023

@m0ksem I'm not sure if we need windows support for development right now.

They have wery nice wsl (something I personally used to avoid getting confused on random errors).

I'd say we might want to handle that would we have more resources. As you have to pretty much test all scripts to make sure this works. Expanded the topic here - #4029

@asvae
Copy link
Member

asvae commented Nov 22, 2023

That's a lot of commits. Feels like a lot of effort 🤗.

@asvae
Copy link
Member

asvae commented Nov 22, 2023

I would prefer camelCase for props as well, but vue styleguide disagrees strongly 😞. So kebab in template that is.

Copy link
Member

@asvae asvae left a comment

Choose a reason for hiding this comment

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

Thanks!

@asvae asvae merged commit 57096a2 into epicmaxco:develop Nov 22, 2023
2 checks passed
@xiongmao86
Copy link
Contributor Author

Thank you for helping me with this pr, @asvae, @m0ksem.

@xiongmao86 xiongmao86 deleted the fix/docs-component-name-casing branch November 23, 2023 12:13
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.

Inconsistent component naming in examples (va-component vs VaComponent)
3 participants