Allow `no-undef` also on plain JavaScript files, not just Typescript
stefanobartoletti opened this issue · 12 comments
Currently the no-undef rule, turned off because of the Nuxt auto-import features, is still set as error on plain JavaScript files, while it is correctly disabled in the specific Typescript configuration.
Since the auto-import feature is also possible on plain JS, I suggest turning this rule off globally, or at least for composables as the bare minimum (i.e. according to the composables/**/*.js pattern).
I'm not sure if this rule should be placed in the same place referenced above (as it seems to be only related to typescript configuration), probably packages/eslint-config/src/flat/configs/javascript.ts is the correct place to set this.
I am available to send a PR myself if this is an acceptable edit.
Turning this rule off globally is in my opinion quite bad. That means you can make mistakes and introduce undefined variables (which aren't auto imported) and you'll be none the wiser.
On the client side, I don't think no-undef is disabled, and there is an explicit list of auto imports that gets generated for you.
On the server, this doesn't seem to happen, and my ESLint is thus complaining about things like defineEventHandler and readBody being undefined. Why have these not been added to the list of globals in this module?
I understand your concerns, but I think I don't agree 100% with you about this, at least for what concerns auto-imports.
For other types, you are totally right, no-undef is better to be in-place, but for functions, Nuxt is already allowing auto-imports basically everywhere (not just in composables, but also, as an example of what I actually used in a recent project, in plugins,stores, and so on), and it is the default behavior, you must explicitly turn this off if you don't want to use this feature: https://nuxt.com/docs/guide/concepts/auto-imports#disabling-auto-imports
Also, according to the docs, there is nothing that implies that auto-imports are only available when using TS and not in plain JS.
So, it is strange that eslint is reporting errors when the framework itself is working just like it is supposed to work, and it behaves like intended.
Maybe there can be an option to set no-undef as error if auto-imports are turned off, but the default setting should conform to Nuxt's default behavior, and at the moment it is not even consistent between TS and JS (this is why I think that it is just an oversight)
It is possible that it was disabled only for TS because, according to this https://eslint.org/docs/latest/rules/no-undef#handled_by_typescript, TS already handles this situation internally, but this leaves an odd situation where if you use default auto-import feature with plain JS, everything works correctly but eslint is throwing errors.
I don't know if there is some way to set no-undef only for functions and leave it in place for every other type (it does not seems so in the basic eslint rule).
Let's wait for some insight from the maintainers, but it seems that either way the current behavior has to be tweaked.
no-undef is disabled in TS because TS already handles that (you should get a TS error instead). I don't see it's necessary to have it in TS.
no-undef should work in JS with auto-import, as the auto imported entry should be augmented in the globals option ESLint: https://github.com/nuxt/eslint/blob/main/packages/module/src/modules/config/addons/globals.ts
Do you mean what you get ESLint error of auto-imported entries in JS? If so, could you share a minimal reproduction? Thank you
I have logged the output of globals, and it didn't include entries from the server, like defineNuxtConfig, or defineEventHandler. I've had to manually add these to globals in my linter config:
globals: {
...globals.builtin,
...globals.browser,
...globals.node,
defineNuxtConfig: 'readonly',
defineEventHandler: 'readonly',
},Front-end/vue related globals were correctly being loaded.
Will try to set up a reproduction later this week.
In my case, I had errors when using auto-imports of custom functions within composables. I will try to reproduce and isolate the problem
Referencing #437 (comment), and using the same steps as a reproduction, but adding yarn add -D typescript to fix the missing dependency problem, I have modified the eslint.config.mjs file to log the globals that are defined:
import js from '@eslint/js'
import vue from 'eslint-plugin-vue'
import withNuxt from './.nuxt/eslint.config.mjs'
const cfg = await withNuxt(
js.configs.recommended,
...vue.configs['flat/recommended'],
{
ignores: [
'*.ts',
'node_modules',
'public',
],
languageOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
},
rules: {
//Because auto imports -.-
'no-undef': 'off',
},
}
)
cfg.forEach(rule => {
console.log(rule.name)
console.log(rule.languageOptions?.globals)
})
export default cfgThis is the output (I have omitted nuxt/javascript and other standard browser/built-in globals for brevity:
nuxt/vue/setup
{
computed: 'readonly',
defineEmits: 'readonly',
defineExpose: 'readonly',
defineProps: 'readonly',
onMounted: 'readonly',
onUnmounted: 'readonly',
reactive: 'readonly',
ref: 'readonly',
shallowReactive: 'readonly',
shallowRef: 'readonly',
toRef: 'readonly',
toRefs: 'readonly',
watch: 'readonly',
watchEffect: 'readonly'
}
nuxt/configs
{ '$fetch': 'readonly' }
nuxt/import-globals
{
useConsentScriptTrigger: 'readonly',
useAnalyticsPageEvent: 'readonly',
useElementScriptTrigger: 'readonly',
useScript: 'readonly',
useScriptGoogleAnalytics: 'readonly',
useScriptPlausibleAnalytics: 'readonly',
useScriptClarity: 'readonly',
useScriptCloudflareWebAnalytics: 'readonly',
useScriptFathomAnalytics: 'readonly',
useScriptMatomoAnalytics: 'readonly',
useScriptGoogleTagManager: 'readonly',
useScriptGoogleAdsense: 'readonly',
useScriptSegment: 'readonly',
useScriptMetaPixel: 'readonly',
useScriptXPixel: 'readonly',
useScriptIntercom: 'readonly',
useScriptHotjar: 'readonly',
useScriptStripe: 'readonly',
useScriptLemonSqueezy: 'readonly',
useScriptVimeoPlayer: 'readonly',
useScriptYouTubePlayer: 'readonly',
useScriptGoogleMaps: 'readonly',
useScriptNpm: 'readonly',
isVue2: 'readonly',
isVue3: 'readonly',
defineNuxtLink: 'readonly',
useNuxtApp: 'readonly',
tryUseNuxtApp: 'readonly',
defineNuxtPlugin: 'readonly',
definePayloadPlugin: 'readonly',
useRuntimeConfig: 'readonly',
defineAppConfig: 'readonly',
requestIdleCallback: 'readonly',
cancelIdleCallback: 'readonly',
setInterval: 'readonly',
useAppConfig: 'readonly',
updateAppConfig: 'readonly',
defineNuxtComponent: 'readonly',
useAsyncData: 'readonly',
useLazyAsyncData: 'readonly',
useNuxtData: 'readonly',
refreshNuxtData: 'readonly',
clearNuxtData: 'readonly',
useHydration: 'readonly',
callOnce: 'readonly',
useState: 'readonly',
clearNuxtState: 'readonly',
clearError: 'readonly',
createError: 'readonly',
isNuxtError: 'readonly',
showError: 'readonly',
useError: 'readonly',
useFetch: 'readonly',
useLazyFetch: 'readonly',
useCookie: 'readonly',
refreshCookie: 'readonly',
onPrehydrate: 'readonly',
prerenderRoutes: 'readonly',
useRequestHeader: 'readonly',
useRequestHeaders: 'readonly',
useRequestEvent: 'readonly',
useRequestFetch: 'readonly',
setResponseStatus: 'readonly',
onNuxtReady: 'readonly',
preloadComponents: 'readonly',
prefetchComponents: 'readonly',
preloadRouteComponents: 'readonly',
abortNavigation: 'readonly',
addRouteMiddleware: 'readonly',
defineNuxtRouteMiddleware: 'readonly',
setPageLayout: 'readonly',
navigateTo: 'readonly',
useRoute: 'readonly',
useRouter: 'readonly',
isPrerendered: 'readonly',
loadPayload: 'readonly',
preloadPayload: 'readonly',
definePayloadReducer: 'readonly',
definePayloadReviver: 'readonly',
useLoadingIndicator: 'readonly',
getAppManifest: 'readonly',
getRouteRules: 'readonly',
reloadNuxtApp: 'readonly',
useRequestURL: 'readonly',
usePreviewMode: 'readonly',
useId: 'readonly',
useRouteAnnouncer: 'readonly',
onBeforeRouteLeave: 'readonly',
onBeforeRouteUpdate: 'readonly',
withCtx: 'readonly',
withDirectives: 'readonly',
withKeys: 'readonly',
withMemo: 'readonly',
withModifiers: 'readonly',
withScopeId: 'readonly',
onActivated: 'readonly',
onBeforeMount: 'readonly',
onBeforeUnmount: 'readonly',
onBeforeUpdate: 'readonly',
onDeactivated: 'readonly',
onErrorCaptured: 'readonly',
onMounted: 'readonly',
onRenderTracked: 'readonly',
onRenderTriggered: 'readonly',
onServerPrefetch: 'readonly',
onUnmounted: 'readonly',
onUpdated: 'readonly',
computed: 'readonly',
customRef: 'readonly',
isProxy: 'readonly',
isReactive: 'readonly',
isReadonly: 'readonly',
isRef: 'readonly',
markRaw: 'readonly',
proxyRefs: 'readonly',
reactive: 'readonly',
readonly: 'readonly',
ref: 'readonly',
shallowReactive: 'readonly',
shallowReadonly: 'readonly',
shallowRef: 'readonly',
toRaw: 'readonly',
toRef: 'readonly',
toRefs: 'readonly',
triggerRef: 'readonly',
unref: 'readonly',
watch: 'readonly',
watchEffect: 'readonly',
watchPostEffect: 'readonly',
watchSyncEffect: 'readonly',
isShallow: 'readonly',
effect: 'readonly',
effectScope: 'readonly',
getCurrentScope: 'readonly',
onScopeDispose: 'readonly',
defineComponent: 'readonly',
defineAsyncComponent: 'readonly',
resolveComponent: 'readonly',
getCurrentInstance: 'readonly',
h: 'readonly',
inject: 'readonly',
hasInjectionContext: 'readonly',
nextTick: 'readonly',
provide: 'readonly',
defineModel: 'readonly',
defineOptions: 'readonly',
defineSlots: 'readonly',
mergeModels: 'readonly',
toValue: 'readonly',
useModel: 'readonly',
useAttrs: 'readonly',
useCssModule: 'readonly',
useCssVars: 'readonly',
useSlots: 'readonly',
useTransitionState: 'readonly',
Component: 'readonly',
ComponentPublicInstance: 'readonly',
ComputedRef: 'readonly',
ExtractPropTypes: 'readonly',
ExtractPublicPropTypes: 'readonly',
InjectionKey: 'readonly',
PropType: 'readonly',
Ref: 'readonly',
MaybeRef: 'readonly',
MaybeRefOrGetter: 'readonly',
VNode: 'readonly',
injectHead: 'readonly',
useHead: 'readonly',
useSeoMeta: 'readonly',
useHeadSafe: 'readonly',
useServerHead: 'readonly',
useServerSeoMeta: 'readonly',
useServerHeadSafe: 'readonly'
}
As you can see, things like defineEventHandler and even defineNuxtConfig are missing from the list of globals, causing ESLint to error when those functions are used.
Hope this helps pinpoint it down.
@antfu this is my minimal reproduction https://github.com/stefanobartoletti/nuxt-eslint-test
If you run pnpm lint, you get this
> pnpm lint
> nuxt-app@ lint /home/stefano/Sviluppo/Test/nuxt-eslint-test
> eslint .
/home/stefano/Sviluppo/Test/nuxt-eslint-test/composables/useGreetNameJs.js
2:24 error 'useGreetJs' is not defined no-undef
3:24 error 'useGreetTs' is not defined no-undef
✖ 2 problems (2 errors, 0 warnings)
ELIFECYCLE Command failed with exit code 1.
As explained in the original post, it concerns user composables, and especially using them inside other composables when they are in Js and not Ts. But I think that simply running the demo is more understandable than try to describe it, feel free to ask if something is not clear of course.
Note that in this case I'm getting errors when trying to use composables inside another Js composable, and I wanted to keep the demo as minimal as possible, but I'm getting the same issue even if I try to auto-import composables from plugins, stores, and so on, if I import them in Javascript files.
@adamreisnz I appreciate your contribution to the subject, but while we are referring to the same issue more or less, there are different nuances (local functions vs builtin) and it seems that the discussion is taking two different paths. For the sake of understandability and to better follow replies, probably it would be better at this point to have two separate issue threads.
Let's hear from the maintainers, but if they think this could be a good idea, would you mind opening a new issue with your specific configuration? Thanks :)
@stefanobartoletti You are not using the module - that explains why because they are never get generated. What blocks you from using the module?
@antfu I used the config and not the module, because I wanted to integrate it on my personal config, where I have some preferences about code regardless of the underlying framework.
But probably this is not the best way to put everything together I suppose. Is the module the only way to have this functionality?
Yes, because it needs to get all this information from the Nuxt instance. You can use https://eslint.nuxt.com/packages/module#custom-config-presets to disable the built-in presets and leave only the nuxt-specific rules.
Consider this is not a bug, closing it for now. If you have any better idea to resolve this, feel free to bring up in another issue. Thanks
@antfu Thanks for the information, I don't think that it will be necessary to bring it up again, I had included some custom rules for Nuxt in my setup because the old package was not reliable enough.
But now I think it is better to refactor my config and make it work within the Nuxt official package and not the other way around.
Thanks for your support ✌️
I've created #461 with a reproduction for my issue of undefined methods since this ticket was closed.