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

eslint rule no-document-import-in-page false positive in _document.test #28169

Closed
ljosberinn opened this issue Aug 16, 2021 · 1 comment · Fixed by #28148
Closed

eslint rule no-document-import-in-page false positive in _document.test #28169

ljosberinn opened this issue Aug 16, 2021 · 1 comment · Fixed by #28148
Labels
bug Issue was opened via the bug report template.

Comments

@ljosberinn
Copy link
Contributor

What version of Next.js are you using?

11.1.0

What version of Node.js are you using?

latest

What browser are you using?

irrelevant

What operating system are you using?

win

How are you deploying your application?

irrelevant

Describe the Bug

Given a file called _document in any folder outside of pages, the rule will crash:

  create: function (context) {
    return {
      ImportDeclaration(node) {
        if (node.source.value !== 'next/document') {
          return
        }

        const page = context.getFilename().split('pages')[1] // <--- crashes
        const { name, dir } = path.parse(page)

context.getFilename() in my case is C:\Users\gerr.it\Desktop\dev\personal-react-boilerplate\src\client\__tests__\_document.test.tsx

error with stack trace:

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:329:5)
    at validateString (node:internal/validators:129:11)
    at Object.parse (node:path:853:5)
    at ImportDeclaration (C:\Users\gerr.it\Desktop\dev\personal-react-boilerplate\node_modules\@next\eslint-plugin-next\lib\rules\no-document-import-in-page.js:20:36)

Proposed solution:

  create: function (context) {
    return {
      ImportDeclaration(node) {
        if (node.source.value !== 'next/document') {
          return;
        }

        const page = context.getFilename().split('pages')[1];

        if (!page) {
          return;
        }

        const { name, dir } = path.parse(page);

        if (
          name.startsWith('_document') ||
          name.includes('.test') ||
          (dir === '/_document' && name === 'index')
        ) {
          return;
        }

Expected Behavior

works as expected

To Reproduce

have an import on anything from next/document outside of the pages folder should crash

@ljosberinn ljosberinn added the bug Issue was opened via the bug report template. label Aug 16, 2021
@kodiakhq kodiakhq bot closed this as completed in #28148 Aug 16, 2021
kodiakhq bot pushed a commit that referenced this issue Aug 16, 2021
Fixes #28030 
Fixes #28169

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants