-
Notifications
You must be signed in to change notification settings - Fork 151
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
Test files included in release tarball and module.exports
based on content of directory are messing with esbuild
bundler
#466
Comments
I have the same issue I temporarily fixed it by adding |
Same issue. On nextjs: Module not found: Can't resolve 'tap'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/pino-pretty/lib/utils/ sync ^\.\/.*\.js$
./node_modules/pino-pretty/lib/utils/index.js
./node_modules/pino-pretty/index.js
./node_modules/pino/lib/tools.js
./node_modules/pino/pino.js
./node_modules/@walletconnect/logger/dist/cjs/index.js
./node_modules/@walletconnect/universal-provider/dist/index.es.js
./node_modules/@walletconnect/ethereum-provider/dist/index.es.js
./node_modules/@wagmi/connectors/dist/walletConnect.js
./node_modules/@wagmi/core/dist/connectors/walletConnect.js
./node_modules/wagmi/dist/connectors/walletConnect.js
./node_modules/@rainbow-me/rainbowkit/dist/index.js
./app/page.tsx Try fix with ./node_modules/@tapjs/stack/dist/commonjs/require-resolve.js
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted
Import trace for requested module:
./node_modules/@tapjs/stack/dist/commonjs/require-resolve.js
./node_modules/@tapjs/stack/dist/commonjs/index.js
./node_modules/@tapjs/core/dist/commonjs/extra-from-error.js
./node_modules/@tapjs/core/dist/commonjs/index.js
./node_modules/tap/dist/commonjs/main.js
./node_modules/tap/dist/commonjs/index.js
./node_modules/pino-pretty/lib/utils/build-safe-sonic-boom.test.js
./node_modules/pino-pretty/lib/utils/ sync ^\.\/.*\.js$
./node_modules/pino-pretty/lib/utils/index.js
./node_modules/pino-pretty/index.js
./node_modules/pino/lib/tools.js
./node_modules/pino/pino.js
./node_modules/@walletconnect/logger/dist/cjs/index.js
./node_modules/@walletconnect/universal-provider/dist/index.es.js
./node_modules/@walletconnect/ethereum-provider/dist/index.es.js
./node_modules/@wagmi/connectors/dist/walletConnect.js
./node_modules/@wagmi/core/dist/connectors/walletConnect.js
./node_modules/wagmi/dist/connectors/walletConnect.js
./node_modules/@rainbow-me/rainbowkit/dist/index.js
./app/page.tsx |
In my opinion, this is a problem with the tooling you are using. This module is intended to be used within a Node.js environment. Such an environment should have no issue with the code in question. |
@jsumners I think we should refactor that block, mostly because quite a few people bundle their server dependencies and it would not work there. It also loads slightly slower than just listing all files manually. |
I don't care for that approach because it requires ongoing maintenance. But the Lines 15 to 18 in 9fb7330
I won't block a PR that solve the issue either way. |
Considering that I do not see any complex tool configurations in your repository, I assume you are creating the npm tarball by running # Test files
*.test.js
test/
# Example images
*.png
# Other files only used in development
.*
benchmark.js
coverage-map.js
tsconfig.json will not only exclude test files, but also some other development-only files, from being published to npm. This will also reduce the tarball size:
Let me know what you think. I can create a PR with that change later today. |
Please see the discussion in fastify/skeleton#42 (comment) for my reasoning against The changes that will solve this are either of:
|
Understandable. One could add some fancy
I can implement this later. |
* Fixes #466 Signed-off-by: Marvin A. Ruder <signed@mruder.dev>
Release version 10.2.2 contains the following code which dynamically processes all JavaScript files in the same directory, supposedly excluding test files:
pino-pretty/lib/utils/index.js
Lines 6 to 17 in 9fb7330
However, not all bundlers can handle this code. ESBuild for example describes their capability of bundling glob-style imports here. Since
it is unable to detect that
*.test.js
files are not to be bundled.This leads to errors such as:
This is easily reproducable using the following minimal setup:
package.json
:index.js
:Running
find node_modules/pino-pretty -name "*.test.js" -delete
solves the issue.Would it be possible to not ship
.test.js
files with pino-pretty, e.g. by removing them as part your build and release pipeline?The text was updated successfully, but these errors were encountered: