unjs / unbuild

📦 A unified JavaScript build system
MIT License
2.31k stars 90 forks source link

`declaration` option is not work when `package.json` set `exports.types` #380

Closed s3xysteak closed 6 months ago

s3xysteak commented 6 months ago

Environment

Reproduction

See minirepo.

Describe the bug

The option declaration do not do anything because in the end it will be set according to exports in package.json.

Describe Bug

declaration: false will not work:

import { defineBuildConfig } from 'unbuild'

export default defineBuildConfig({
  declaration: false,
})

When in package.json:

{
  "exports": {
    "types": "./dist/index.d.ts",
    "import": "./dist/index.mjs",
    "require": "./dist/index.cjs"
  },
}

The declaration will be set true if the values of exports contains any files named *.d.(m|c)?ts in package.json.

Also, because of the same reason, declaration: 'node16' can not work too, it is same as true instead of the description in comments.

More details refer to:

https://github.com/unjs/unbuild/blob/bfb4a348421826ad48072ca1cf70973e6caa5b1d/src/auto.ts#L34

https://github.com/unjs/unbuild/blob/bfb4a348421826ad48072ca1cf70973e6caa5b1d/src/auto.ts#L154

Discussion

The problem is whether it is a bug or a deliberate design. I think when declaration is set as false explicitly, it should be false. It is counter-intuitive that set it as false but it still work. So in my option, user-initiated actions should have the highest priority.

Additional context

To fix it, refer to:

https://github.com/unjs/unbuild/blob/bfb4a348421826ad48072ca1cf70973e6caa5b1d/src/auto.ts#L34

If declaration is not undefined, it should use what user provided.

I am glad to provide a PR for that!

Logs

No response

s3xysteak commented 6 months ago

Duplicate #339 #313. Will be fixed in #314 .