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.

Sign up for GitHub

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

fix: change import to default in package exports #1028

Merged
merged 4 commits into from
Feb 28, 2023
+19 −12
Merged

fix: change import to default in package exports #1028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ecbbd50
revert: package types exports for ESM projects
cseas Feb 28, 2023
fb9f659
docs: changeset
cseas Feb 28, 2023
867e95b
fix: change import to default
cseas Feb 28, 2023
1b7c3b6
docs: update changeset
cseas Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/two-carrots-provide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@razorpay/blade': patch
---

fix: change import to default in package exports

Jest does not support the "import" condition in exports. This was causing tests to fail for Blade consumers. Changed "import" to "default" which is supported by all tools. Since Blade is not exporting a dual package, we don't need the "import" condition.
24 changes: 12 additions & 12 deletions packages/blade/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,32 +29,32 @@
"exports": {
"./components": {
"react-native": {
"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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you link what is the diff between using default and import here?

Copy link
Contributor Author

@cseas cseas Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

},
"default": {
"import": "./build/components/index.web.js",
"types": "./build/components/index.d.ts"
"types": "./build/components/index.d.ts",
"default": "./build/components/index.web.js"
}
},
"./tokens": {
"react-native": {
"import": "./build/tokens/index.native.js",
"types": "./build/tokens/index.native.d.ts"
"types": "./build/tokens/index.native.d.ts",
"default": "./build/tokens/index.native.js"
},
"default": {
"import": "./build/tokens/index.web.js",
"types": "./build/tokens/index.d.ts"
"types": "./build/tokens/index.d.ts",
"default": "./build/tokens/index.web.js"
}
},
"./utils": {
"react-native": {
"import": "./build/utils/index.native.js",
"types": "./build/utils/index.native.d.ts"
"types": "./build/utils/index.native.d.ts",
"default": "./build/utils/index.native.js"
},
"default": {
"import": "./build/utils/index.web.js",
"types": "./build/utils/index.d.ts"
"types": "./build/utils/index.d.ts",
"default": "./build/utils/index.web.js"
}
}
},
Expand Down