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

False positive in import plugin #3157

Open
Boshen opened this issue May 4, 2024 · 9 comments
Open

False positive in import plugin #3157

Boshen opened this issue May 4, 2024 · 9 comments
Assignees
Labels
C-bug Category - Bug

Comments

@Boshen
Copy link
Member

Boshen commented May 4, 2024

These contain type exports

https://github.com/palantir/blueprint

blueprint  develop ❯ ~/github/oxc/target/release/oxlint --import-plugin -D export

  ⚠ eslint-plugin-import(export): No named exports found in module './itemRenderer'
    ╭─[packages/select/src/common/index.ts:21:15]
 20 │ export * from "./itemListRenderer";
 21 │ export * from "./itemRenderer";
    ·               ────────────────
 22 │ export * from "./listItemsProps";
    ╰────

  ⚠ eslint-plugin-import(export): No named exports found in module './predicate'
    ╭─[packages/select/src/common/index.ts:24:15]
 23 │ export * from "./listItemsUtils";
 24 │ export * from "./predicate";
    ·               ─────────────
 25 │ export type { SelectPopoverProps } from "./selectPopoverProps";
    ╰────

  ⚠ eslint-plugin-import(export): Multiple exports of name 'TimePrecision'.
    ╭─[packages/datetime/src/index.ts:29:10]
 28 │ export type { TimePickerProps } from "./common/timePickerProps";
 29 │ export { TimePrecision } from "./common/timePrecision";
    ·          ─────────────
 30 │
    ╰────
@Boshen Boshen added the C-bug Category - Bug label May 4, 2024
@Boshen Boshen self-assigned this May 4, 2024
@Boshen
Copy link
Member Author

Boshen commented May 4, 2024

blueprint  develop ❯ ~/github/oxc/target/release/oxlint --import-plugin -D named

  ⚠ eslint-plugin-import(named): named import "default" not found
   ╭─[packages/karma-build-scripts/index.mjs:7:10]
 6 │
 7 │ export { default as createKarmaConfig } from "./createKarmaConfig.mjs";
   ·          ───────
   ╰────
  help: does "./createKarmaConfig.mjs" have the export "default"?

  ⚠ eslint-plugin-import(named): named import "default" not found
    ╭─[packages/webpack-build-scripts/index.mjs:19:10]
 18 │ export { COMMON_EXTERNALS } from "./externals.mjs";
 19 │ export { default as baseConfig } from "./webpack.config.base.mjs";
    ·          ───────
 20 │ export { default as karmaConfig } from "./webpack.config.karma.mjs";
    ╰────
  help: does "./webpack.config.base.mjs" have the export "default"?

  ⚠ eslint-plugin-import(named): named import "default" not found
    ╭─[packages/webpack-build-scripts/index.mjs:20:10]
 19 │ export { default as baseConfig } from "./webpack.config.base.mjs";
 20 │ export { default as karmaConfig } from "./webpack.config.karma.mjs";
    ·          ───────
    ╰────
  help: does "./webpack.config.karma.mjs" have the export "default"?

@Boshen Boshen changed the title False positive in import plugin's export False positive in import plugin May 4, 2024
@Dunqing Dunqing assigned Dunqing and unassigned Boshen May 4, 2024
@Boshen
Copy link
Member Author

Boshen commented May 4, 2024

@Dunqing What should be the best way to fix this? Save all the export names for TypeScript types?

@Boshen
Copy link
Member Author

Boshen commented May 4, 2024

export type * from "mod" doesn't fix the error 😅

@Dunqing
Copy link
Member

Dunqing commented May 4, 2024

@Dunqing What should be the best way to fix this? Save all the export names for TypeScript types?

Yes, it seems we must. But after doing so we also need to deal with type merging. Such as allowing multiple namespaces with the same name, and allowing export const A and export type A with the same name.

@Dunqing
Copy link
Member

Dunqing commented May 4, 2024

  ⚠ eslint-plugin-import(export): Multiple exports of name 'TimePrecision'.
    ╭─[packages/datetime/src/index.ts:29:10]
 28 │ export type { TimePickerProps } from "./common/timePickerProps";
 29 │ export { TimePrecision } from "./common/timePrecision";
    ·          ─────────────
 30 │
    ╰────

I double-checked this error and it's not a false positive. TimePrecision is also exported in common/index.ts. This file has export * from ". /common"

@Boshen
Copy link
Member Author

Boshen commented May 4, 2024

  ⚠ eslint-plugin-import(export): Multiple exports of name 'TimePrecision'.
    ╭─[packages/datetime/src/index.ts:29:10]
 28 │ export type { TimePickerProps } from "./common/timePickerProps";
 29 │ export { TimePrecision } from "./common/timePrecision";
    ·          ─────────────
 30 │
    ╰────

I double-checked this error and it's not a false positive. TimePrecision is also exported in common/index.ts. This file has export * from ". /common"

How can we improve the error message 🤔

@Boshen
Copy link
Member Author

Boshen commented May 4, 2024

@Dunqing What should be the best way to fix this? Save all the export names for TypeScript types?

Yes, it seems we must. But after doing so we also need to deal with type merging. Such as allowing multiple namespaces with the same name, and allowing export const A and export type A with the same name.

We can add a exported_typings to ModuleRecord, instead of mixing everything with the existing bindings.

@Boshen
Copy link
Member Author

Boshen commented May 6, 2024

https://github.com/rolldown/rolldown/actions/runs/8968666129/job/24628561631?pr=1027

> monorepo@ lint-code /home/runner/work/rolldown/rolldown
> oxlint --ignore-path=./.oxlintignore --import-plugin --deny-warnings


  ! eslint-plugin-import(named): named import "default" not found
   ,-[examples/par-babel-rome-ts/rolldown.config.js:5:10]
 4 | import nodeUrl from 'node:url'
 5 | import { default as parallelBabelPluginSync } from './parallel-babel-plugin/index.js'
   :          ^^^^^^^
 6 | 
   `----
  help: does "./parallel-babel-plugin/index.js" have the export "default"?

  ! eslint-plugin-import(default): No default export found in imported module "./CodeBlock.vue"
   ,-[web/playground/src/components/ModuleBlock.vue:2:[8](https://github.com/rolldown/rolldown/actions/runs/8968666129/job/24628561631?pr=1027#step:6:9)]
 1 | 
 2 | import CodeBlock from './CodeBlock.vue'
   :        ^^^^^^^^^
 3 | import { onMounted, ref } from 'vue'
   `----
  help: does "./CodeBlock.vue" have the default export?

  ! eslint-plugin-import(default): No default export found in imported module "./App.vue"
   ,-[web/playground/src/main.ts:5:8]
 4 | import VueCodemirror from 'vue-codemirror'
 5 | import App from './App.vue'
   :        ^^^
 6 | 
   `----
  help: does "./App.vue" have the default export?

  ! eslint-plugin-import(named): named import "default" not found
   ,-[packages/bench/src/suites/rome-ts.js:7:3]
 6 | import {
 7 |   default as parallelBabelPluginAsync,
   :   ^^^^^^^
 8 |   syncVersion as parallelBabelPluginSync,
   `----
  help: does "../parallel-babel-plugin/index.js" have the export "default"?

@Boshen
Copy link
Member Author

Boshen commented May 9, 2024

@Dunqing I tested locally, they aren't fixed yet with --ignore-path=./.oxlintignore --import-plugin --deny-warnings https://github.com/rolldown/rolldown/actions/runs/9011082137/job/24758169189?pr=1065

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

No branches or pull requests

2 participants