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

It's unreasonable to trigger Inlined implicit external warning when the related alias was configured. #201

Closed
Plasticine-Yang opened this issue Jan 17, 2023 · 18 comments

Comments

@Plasticine-Yang
Copy link

Plasticine-Yang commented Jan 17, 2023

Environment

Node.js version: v18.12.1

Reproduction

https://github.com/Plasticine-Yang/plasticine-monitor

run pnpm build:browser can trigger the bug.

Describe the bug

It will trigger Inlined implicit external @/sdk when I don't config the alias for @, which is reasonable.

However, the @/sdk is a path alias point to packages/browser/src/sdk.ts, it is not a dependency package.

I also configured the paths option in tsconfig.json, but it is just for IDE and tsc to resolve the path, which is not related to the rollup under the unbuild.

So, it should be solved when I configure the alias @ pointing to packages/browser/src, but it still trigger the Inlined implicit external @/sdk warning.

I'm not sure whether it is a bug or a feature. The issue can be closed if it is a feature.

Additional context

The file imports a module with alias: packages/browser/src/plugins/error-monitor-plugin/index.ts:

import type { Plugin } from '@plasticine-monitor/core'

import { WebSDK } from '@/sdk'
import { monitorJSError } from './monitors/js-error'

/**
 * @description 监听运行时错误插件
 */
const errorMonitorPlugin = (): Plugin<WebSDK> => {
  return {
    install(webSDK) {
      monitorJSError(webSDK)
    },
  }
}

const __TEMP__ = WebSDK
console.log(__TEMP__)

export { errorMonitorPlugin }

The @/sdk points to packages/browser/src/sdk.ts.

build.config.ts

export default defineBuildConfig({
  entries: ['./src/index'],
  outDir: 'dist',
  clean: true,
  declaration: true,
  rollup: {
    emitCJS: true,
  },
  alias: {
    '@': resolve(__dirname, 'src'),
  },
})

image

Logs

No response

@sechi747
Copy link

Same for me, have you resolved it?

@Plasticine-Yang
Copy link
Author

Same for me, have you resolved it?

Not yet. No one responded to me, I could only cancel the alias.

@sechi747
Copy link

sechi747 commented Feb 13, 2023

Same for me, have you resolved it?

Not yet. No one responded to me, I could only cancel the alias.

I just add failOnWarn: false in build.config.ts, because the packaged files are correct. It just a warning not an error.

@NamesMT
Copy link

NamesMT commented Nov 23, 2023

+1 simple aliases like ~/ for src/ also trigger "Inlined implicit external", this with the fact that alias breaks in stub mode and mkdist means unbuild doesn't support alias at all.

NamesMT pushed a commit to NamesMT/starter-ts that referenced this issue Nov 23, 2023
Use relative path in index because of unjs/unbuild#201
Remove abundant packages
Rebuilt lockfile & nolyfill
Template test should no longer throws when replace the template index
@pi0
Copy link
Member

pi0 commented Nov 23, 2023

It should not happen if aliases are configured to be bundled or externalized. Please provide a minimal reproduction (ie stackblitz/codesandbox).

NamesMT added a commit to NamesMT/starter-ts that referenced this issue Nov 24, 2023
@NamesMT
Copy link

NamesMT commented Nov 24, 2023

Hello @pi0, heres a reproduction link:
https://codesandbox.io/p/github/NamesMT/starter-ts/draft/musing-albattani (run the reproduce script)

The warning is triggered using normal bundle mode and not mkdist, (but mkdist also breaks), alias is configured in both build.config.ts and tsconfig.json

@Plasticine-Yang
Copy link
Author

Same for me, have you resolved it?

I just replace ungit with tsup.

@gnclmorais
Copy link

I ran into the same issue… :/

@RIP21
Copy link

RIP21 commented Mar 10, 2024

Even for simple aliases but with the mkdist builder it won't work. Let's say @lib alias to lib folder in case of mkdist doesn't work and stays @lib/whatever in the imports of the emitted files.

For anyone who is looking for a workaround, the only one I figured is to use vite in lib mode (config below). It's slower in case of the first run (13-15 seconds for my lib) (but very reliable as it uses tsc to emit declarations) but subsequent if you use vite build --watch is 8-9x faster. So just 2-3 seconds for my project.

// @ts-expect-error
import peerDepsExternal from 'rollup-plugin-peer-deps-external'
import type { LibraryFormats, UserConfig } from 'vite'
import dts from 'vite-plugin-dts'
import noBundle from 'vite-plugin-no-bundle'
import tsconfigPaths from 'vite-tsconfig-paths'
import react from '@vitejs/plugin-react-swc'

export default defineConfig({
  {
    plugins: [
      tsconfigPaths(),
      react(),
      dts({ include: ['lib'], insertTypesEntry: true }),
      noBundle({
        root: './lib',
      }),
    ],
    build: {
      minify: false,
      sourcemap: true,
      copyPublicDir: false,
      reportCompressedSize: false,
      lib: {
        entry: {
          index: resolve(__dirname, 'lib/index.ts')
        },
        formats: ["es"],
      },
      rollupOptions: {
        plugins: [
          peerDepsExternal({
            includeDependencies: true,
          }),
        ],
      },
    },
  }
})

@trandaison
Copy link

@sechi747

I just add failOnWarn: false in build.config.ts, because the packaged files are correct. It just a warning not an error.

Can you explain a little bit more? I am facing with this problem with https://github.com/nuxt/module-builder (which is built on top of unbuild), but there is no documents that show how/where to set failOnWarn: false

@sechi747
Copy link

@trandaison
failOnWarn is a configurable option of unbuild. You can see its type definition here: https://github.com/unjs/unbuild/blob/main/src/types.ts#L87
But the nuxt/module-builder indeed has fixed build options and cannot be freely configured. You can see its build method here: https://github.com/nuxt/module-builder/blob/main/src/commands/build.ts#L34

@trandaison
Copy link

@sechi747 My current workaround solution:

// build.config.ts

import { defineBuildConfig } from "unbuild";

export default defineBuildConfig({
  failOnWarn: false,
});

@HigherOrderLogic
Copy link

Hi @pi0, is this issue on the roadmap? These false positive warnings are very annoying to work with.

NamesMT added a commit to NamesMT/hono-adapter-aws-lambda that referenced this issue Jun 19, 2024
@olets
Copy link

olets commented Sep 13, 2024

The same thing happens with package.json subpath imports https://nodejs.org/api/packages.html#subpath-imports

Here's a minimal reproduction https://stackblitz.com/edit/vitejs-vite-kwryzn?file=src%2Findex.ts

Lmk if I should open a new issue.

@xsjcTony
Copy link

Indeed it's happening for my project as well. https://github.com/xsjcTony/remark-magic-link
This should definitely be fixed as it forces you to set failOnWarn: false which is not safe.

@xsjcTony
Copy link

There's seems to be the same issue before, and it's fixed #383.
Checked the release note, maybe try v3.0.0-rc2?
https://github.com/unjs/unbuild/releases/tag/v3.0.0-rc.2

@wangjue666
Copy link

之前似乎也有同样的问题,已修复#383。 查看了发布说明,也许可以试试v3.0.0-rc2https://github.com/unjs/unbuild/releases/tag/v3.0.0-rc.2

It's useful to me

@pi0
Copy link
Member

pi0 commented Dec 27, 2024

#384 available in 3.x fixes issue with aliases that were wrongly detected as externals.

#351 aims to also auto add package namage/subpaths itself as they are valid aliases in ESM resolution.

In case you had other issues in v3, please feel free to report a new issue 🙏🏼

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

No branches or pull requests