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

warning in package.json during build #3078

Open
ST-DDT opened this issue Aug 31, 2024 · 3 comments
Open

warning in package.json during build #3078

ST-DDT opened this issue Aug 31, 2024 · 3 comments
Assignees
Labels
c: bug Something isn't working c: infra Changes to our infrastructure or project setup

Comments

@ST-DDT
Copy link
Member

ST-DDT commented Aug 31, 2024

There is a warning in the package.json when the code is build using tsup.

Warning: G] The condition "default" here will never be used as it comes after both "import" and "require" [package.json]

    package.json:48:6:
      48 │       "default": "./dist/index.js"
         ╵       ~~~~~~~~~

  The "import" condition comes earlier and will be used for all "import" statements:

    package.json:46:6:
      46 │       "import": "./dist/index.js",
         ╵       ~~~~~~~~

  The "require" condition comes earlier and will be used for all "require" calls:

    package.json:47:6:
      47 │       "require": "./dist/index.cjs",
         ╵       ~~~~~~~~~

Warning: G] The condition "default" here will never be used as it comes after both "import" and "require" [package.json]

    package.json:54:6:
      54 │       "default": "./dist/locale/*.js"
         ╵       ~~~~~~~~~

  The "import" condition comes earlier and will be used for all "import" statements:

    package.json:52:6:
      52 │       "import": "./dist/locale/*.js",
         ╵       ~~~~~~~~

  The "require" condition comes earlier and will be used for all "require" calls:

    package.json:53:6:
      53 │       "require": "./dist/locale/*.cjs",
         ╵       ~~~~~~~~~

Related code:

faker/package.json

Lines 44 to 49 in 17f570e

".": {
"types": "./dist/types/index.d.ts",
"import": "./dist/index.js",
"require": "./dist/index.cjs",
"default": "./dist/index.js"
},

faker/package.json

Lines 50 to 55 in 17f570e

"./locale/*": {
"types": "./dist/types/locale/*.d.ts",
"import": "./dist/locale/*.js",
"require": "./dist/locale/*.cjs",
"default": "./dist/locale/*.js"
},

@ST-DDT ST-DDT added c: bug Something isn't working s: pending triage Pending Triage c: infra Changes to our infrastructure or project setup labels Aug 31, 2024
@Shinigami92
Copy link
Member

Shinigami92 commented Aug 31, 2024

We should be a bit careful about this.
https://nodejs.org/api/packages.html#conditional-exports is showing that there are other environments like "node". I also heard once about "browser" to be a thing.

So if this warning is really by tsup, it might not be fully true 🤔

Edit: Or in other words: please double check if we can safely rewrite this

Edit 2 / Afterthoughts: maybe there is an option to suppress this warning? Or it will be gone in v10 when we only support esm only 🤔

@ST-DDT
Copy link
Member Author

ST-DDT commented Aug 31, 2024

Or is this sometging that we have to report to/ask tsup?

@Shinigami92
Copy link
Member

@xDivisionByZerox xDivisionByZerox removed the s: pending triage Pending Triage label Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working c: infra Changes to our infrastructure or project setup
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants