bunchee icon indicating copy to clipboard operation
bunchee copied to clipboard
verified publisher icon huozhi

`development` / `production` export condition not working inside another condition

Open amannn opened this issue 1 year ago • 6 comments

Thank you so much for adding support for development / production export conditions in v.4.40! I think I noticed a bug.

This works:

  "exports": {
    ".": {
      "development": "./dist/index.development.mjs",
      "production": "./dist/index.production.mjs",
      "default": "./dist/index.js"
    }
  },
Output 
  Exports         File                          Size
 . (production) dist/index.development.mjs    9.27 kB
                dist/index.production.mjs     6.54 kB
                dist/index.js                 9.97 kB
                dist/index.mjs                9.97 kB
                dist/index.d.mts              23.4 kB
                dist/index.production.d.mts   23.4 kB
                dist/index.d.ts               23.4 kB
                dist/index.development.d.mts  23.4 kB

… while this doesn't:

  "exports": {
    ".": {
      "import": {
        "types": "./dist/index.d.mts",
        "development": "./dist/index.development.mjs",
        "production": "./dist/index.production.mjs",
        "default": "./dist/index.mjs"
      },
      "require": {
        "types": "./dist/index.d.ts",
        "production": "./dist/index.production.js",
        "development": "./dist/index.development.js",
        "default": "./dist/index.js"
      },
      "default": "./dist/index.js"
    }
  },
Output 
Exports  File              Size
 . dist/index.mjs    9.97 kB
   dist/index.js     10.5 kB
   dist/index.d.ts   23.4 kB
   dist/index.d.mts  23.4 kB

The webpack docs mention:

Conditions might be nested to create a logical AND.

Is this a bug? Tested in v.4.4.0.

Many thanks!

amannn avatar Jan 15 '24 09:01 amannn

Sounds like a bug, will investigate!

huozhi avatar Jan 15 '24 13:01 huozhi

Seems not a quick fix, would you try if Adding index.development.js as source file works for you? Just re-exports everything from index.js

huozhi avatar Jan 16 '24 22:01 huozhi

Thanks for having a look!

Adding index.development.js as source file works for you?

Unfortunately not, it seems.

  "exports": {
    ".": {
      "import": {
        "types": "./dist/index.d.mts",
        "development": "./dist/index.development.mjs",
        "production": "./dist/index.production.mjs",
        "default": "./dist/index.mjs"
      },
      "require": {
        "types": "./dist/index.d.ts",
        "production": "./dist/index.production.js",
        "development": "./dist/index.development.js",
        "default": "./dist/index.js"
      },
      "default": "./dist/index.js"
    }
  },

src
  src/index.development.tsx
  src/index.production.tsx
  src/index.tsx

Output:

Exports  File              Size
 . dist/index.js     10.5 kB
   dist/index.mjs    9.97 kB
   dist/index.d.ts   23.4 kB
   dist/index.d.mts  23.4 kB

✓ bunchee 4.4.0 build completed

Here's a minimal reproduction if that helps: https://github.com/amannn/bunchee-test

amannn avatar Jan 17 '24 06:01 amannn

@amannn Landed a fix in 4.4.2, would you like to give it another shot? 🙏

huozhi avatar Jan 25 '24 22:01 huozhi

Thanks! I can confirm the snippet from above works now, yes! 🎉

However, I noticed that once I introduce sub-exports apart from ., only the last entry gets a dev and prod build:

  "exports": {
    ".": {
      "import": {
        "types": "./dist/index.d.mts",
        "development": "./dist/index.development.mjs",
        "production": "./dist/index.production.mjs",
        "default": "./dist/index.mjs"
      },
      "require": {
        "types": "./dist/index.d.ts",
        "production": "./dist/index.production.js",
        "development": "./dist/index.development.js",
        "default": "./dist/index.js"
      },
      "default": "./dist/index.js"
    },
    "./core": {
      "import": {
        "types": "./dist/core.d.mts",
        "development": "./dist/core.development.mjs",
        "production": "./dist/core.production.mjs",
        "default": "./dist/core.mjs"
      },
      "require": {
        "types": "./dist/core.d.ts",
        "production": "./dist/core.production.js",
        "development": "./dist/core.development.js",
        "default": "./dist/core.js"
      },
      "default": "./dist/core.js"
    },
    "./react": {
      "import": {
        "types": "./dist/react.d.mts",
        "development": "./dist/react.development.mjs",
        "production": "./dist/react.production.mjs",
        "default": "./dist/react.mjs"
      },
      "require": {
        "types": "./dist/react.d.ts",
        "production": "./dist/react.production.js",
        "development": "./dist/react.development.js",
        "default": "./dist/react.js"
      },
      "default": "./dist/react.js"
    }
  },

Output:

> bunchee

Exports          File                          Size
./ (production)  dist/react.development.mjs    203 B
                 dist/react.production.js      681 B
                 dist/react.development.js     739 B
                 dist/index.js                 783 B
                 dist/index.mjs                247 B
                 dist/react.production.mjs     145 B
                 dist/index.d.mts              123 B
                 dist/index.d.ts               123 B
                 dist/react.development.d.ts   123 B
                 dist/react.production.d.mts   123 B
                 dist/react.development.d.mts  123 B
                 dist/react.production.d.ts    123 B
./core           dist/core.js                  107 B
                 dist/core.mjs                 39 B
                 dist/core.d.ts                47 B
                 dist/core.d.mts               47 B
./react          dist/react.mjs                42 B
                 dist/react.js                 111 B
                 dist/react.d.ts               50 B
                 dist/react.d.mts              50 B

✓ bunchee 4.4.3 build completed

See my updated repro.

I also noticed that somehow the react sub-export gets the code for . (react.development.mjs).

Am I missing something?

amannn avatar Jan 29 '24 08:01 amannn

Looks like need to rewrite a lot of exports parsing, only the last export with dev/prod condition one has been found

huozhi avatar Feb 11 '24 12:02 huozhi

Almost rewrite whole exports parsing in PR #465 with the test covered, gonna release in the next major version 🙏

huozhi avatar Mar 03 '24 20:03 huozhi

@huozhi I've updated my example with [email protected], here's the new build output:

Exports  File                          Size
./core   dist/core.d.undefined         51 B
         dist/core.mjs                 158 B
         dist/core.production.mjs      57 B
         dist/core.development.mjs     114 B
         dist/core.js                  226 B
         dist/core.development.js      182 B
         dist/core.production.js       125 B
         dist/core.development.d.mts   51 B
         dist/core.production.d.mts    51 B
         dist/core.d.ts                51 B
         dist/core.d.mts               51 B
         dist/core.development.d.ts    51 B
         dist/core.production.d.ts     51 B
         dist/core.d.mts               226 B
         dist/core.d.ts                226 B
.        dist/index.d.undefined        109 B
         dist/index.production.mjs     132 B
         dist/index.development.mjs    190 B
         dist/index.mjs                234 B
         dist/index.js                 770 B
         dist/index.development.js     726 B
         dist/index.production.js      668 B
         index.js                      770 B
         index.d.ts                    119 B
         dist/index.development.d.mts  109 B
         dist/index.d.mts              109 B
         dist/index.production.d.mts   109 B
         dist/index.development.d.ts   119 B
         dist/index.production.d.ts    119 B
         dist/index.d.ts               119 B
         dist/index.d.mts              770 B
         dist/index.d.ts               770 B
./react  dist/react.d.undefined        53 B
         dist/react.production.js      129 B
         dist/react.mjs                162 B
         dist/react.development.mjs    118 B
         dist/react.production.mjs     60 B
         dist/react.development.js     187 B
         dist/react.js                 231 B
         dist/react.production.d.ts    53 B
         dist/react.production.d.mts   53 B
         dist/react.development.d.mts  53 B
         dist/react.d.mts              53 B
         dist/react.d.ts               53 B
         dist/react.development.d.ts   53 B
         dist/react.d.mts              231 B
         dist/react.d.ts               231 B

Looking at the output, these look a bit suspicious:

  1. There are entries with .undefined.
  2. There are two entries for dist/core.d.ts and dist/core.d.mts (same for . and react). These artifacts seem to not be type definitions but contain runtime code (dist/core.d.ts, dist/core.d.mts)

Are these bugs?

amannn avatar Mar 04 '24 09:03 amannn

  1. Is that main: "index.js" coming from bunchee --prepare? I think the .undefined is caused by that.
  2. The .d.mts is for .mjs extension. I should remove them after the exports parser refactor, ideally they will be gone in v5 stable. Will keep check

huozhi avatar Mar 04 '24 13:03 huozhi

Ah true, that main entry was outdated. I've fixed it in e961321.

The output still has .d.undefined entries though.

Did you see the output for dist/core.d.ts? Seems like some runtime code accidentally makes it into the declaration file there.

amannn avatar Mar 04 '24 13:03 amannn

That sounds like a bug, will investigate 🙏 appreciate the feedback

huozhi avatar Mar 04 '24 15:03 huozhi

Looks like fixed by #475

Output 

Exports  File                        Size
.        dist/index.mjs              234 B
         dist/index.production.js    668 B
         dist/index.development.mjs  190 B
         dist/index.development.js   726 B
         dist/index.production.mjs   132 B
         dist/index.js               770 B
         dist/index.d.ts             109 B
         dist/index.d.mts            109 B
./core   dist/core.development.mjs   114 B
         dist/core.development.js    182 B
         dist/core.production.mjs    57 B
         dist/core.production.js     125 B
         dist/core.mjs               158 B
         dist/core.js                226 B
         dist/core.d.mts             51 B
         dist/core.d.ts              51 B
./react  dist/react.development.mjs  118 B
         dist/react.mjs              162 B
         dist/react.production.mjs   60 B
         dist/react.development.js   187 B
         dist/react.production.js    129 B
         dist/react.js               231 B
         dist/react.d.ts             53 B
         dist/react.d.mts            53 B

huozhi avatar Mar 08 '24 15:03 huozhi

That's great, thanks @huozhi! Let me know when there's a new beta version and I'll give it a go! 🙂

amannn avatar Mar 08 '24 15:03 amannn

5.0.0-beta.2 is out on npm with the above changes 🙏

huozhi avatar Mar 09 '24 13:03 huozhi

Based on my first tests I think this works now, thanks a bunch!! I'll continue my investigation of switching to bunchee by trying out the new shared modules from https://github.com/huozhi/bunchee/pull/480.

amannn avatar Mar 12 '24 13:03 amannn

Thank you @amannn ! Make sure you're using the latest beta, I patched some follow up fixes for it

huozhi avatar Mar 12 '24 14:03 huozhi