fix: change import to default in package exports #1028
Conversation
cseas
requested review from
chaitanyadeorukhkar,
kamleshchandnani,
anuraghazra,
dhruvdutt,
nashcheez,
divyanshu013 and
saurabhdaware
as code owners
🦋 Changeset detectedLatest commit: 1b7c3b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1b7c3b6:
|
|
✅ PR title follows Conventional Commits specification. |
cseas
changed the title
|
have you tested with all the P&C as you did before? Want to be sure that this doesn't break what was fixed as part of #970 |
|
@kamleshchandnani Tested these combinations in a web project:
Can confirm this is not a breaking change. |
| "import": "./build/components/index.native.js", | ||
| "types": "./build/components/index.native.d.ts" | ||
| "types": "./build/components/index.native.d.ts", | ||
| "default": "./build/components/index.native.js" |
There was a problem hiding this comment.
can you link what is the diff between using default and import here?
There was a problem hiding this comment.
Whenever Blade is used with import (e.g. import { Button } from "@razorpay/blade/components";), the tool processing this import (bundler/IDE/test) will look for the "import" condition in Blade's exports. This is true for all tools that support properly resolving the "exports" field.
Jest only supports "browser", "require" or "default" as mentioned in okta/okta-auth-js#1268 (comment), I couldn't find a docs link explaining this. But Jest's support for the "exports" field is still an open issue: browserify/resolve#222
The difference is that import will only work if the tool processing it supports it, jest doesn't. The default field instead works as "if nothing else resolves before this, finally resolve to this". If a default is not provided, the resolution of the package will fail if the tool doesn't support any of the other defined keys.
Node's conditional exports section explains this in detail:
https://nodejs.org/api/packages.html#conditional-exports
There was a problem hiding this comment.
found it here. Now i understand why jest fails to import because jest still doesn't support importing esm things so we could have done require and import so jest could use require export path and webpack can do import export path. but its fine for now since we don't have cjs and esm builds
The changes made in #970 caused Jest tests to break for Blade consumers.
This is because Jest does not support the "import" condition in package exports.
Proper support for the exports field for Jest is still an open issue:
browserify/resolve#222
But we can fix this just by providing a "default" field which Jest does recognise. Since we're not exporting a dual package build which requires explicit
import/requireconditions, the default field is sufficient for all tools consuming Blade.I've also moved the "types" to be defined first as is recommended by TypeScript. It still works right now in
masterbecause of a bug in TypeScript. Refer: https://arethetypeswrong.github.io/