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

add exports field to package.json #659

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ cypress/screenshots
out
dist/
cypress/fixtures/*.js
cypress/fixtures/*.cjs
!cypress/fixtures/shadow-dom-fixture.js

2 changes: 1 addition & 1 deletion cypress/fixtures/alert-dialog.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ <h1 id="my-dialog-title">Dialog title</h1>
</div>
</div>

<script src="./a11y-dialog.js"></script>
<script src="./a11y-dialog.cjs"></script>
Copy link
Owner

Choose a reason for hiding this comment

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

Is the reason we have to update these local paths because they go strictly through the file system and not at all through the package.json file (therefore ignoring the exports field)?

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 I’m actually not sure about that one. Whatever gets tests to pass works 🤷🏻 (it’s been a while since I’ve used Cypress, but if it works, it works)

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, though, .cjs files should never be in <script> tags, because they won’t be supported in browsers. My guess is that Cypress just doesn’t support ESM, or something.

Copy link
Owner

Choose a reason for hiding this comment

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

Actually this has nothing to do with Cypress per se, it’s just a fixture, so I think I could be using the ESM version here. I’ll try.

</body>

</html>
2 changes: 1 addition & 1 deletion cypress/fixtures/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ <h1 id="my-dialog-title">Dialog title</h1>
<button type="button" id="focus-me">Focus me</button>
</div>

<script src="./a11y-dialog.js"></script>
<script src="./a11y-dialog.cjs"></script>
<script>
document.querySelector('#move-focus-outside').addEventListener('click', () => {
document.querySelector('#focus-me').focus()
Expand Down
2 changes: 1 addition & 1 deletion cypress/fixtures/focus.html
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ <h1 id="focusable-shadow-host-dialog-title">Dialog with an focusable shadow host
<button type="button" id="focus-me">Focus me</button>
</div>

<script src="./a11y-dialog.js"></script>
<script src="./a11y-dialog.cjs"></script>
<script src="./shadow-dom-fixture.js"></script>
<script>
document.querySelector('#move-focus-outside').addEventListener('click', () => {
Expand Down
2 changes: 1 addition & 1 deletion cypress/fixtures/instance.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ <h1 id="my-dialog-title">Dialog title</h1>
</div>
</div>

<script src="./a11y-dialog.js"></script>
<script src="./a11y-dialog.cjs"></script>
</body>

</html>
2 changes: 1 addition & 1 deletion cypress/fixtures/nested-dialogs.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ <h1 id="dialog-title-3">Dialog 3</h1>
</div>
</div>

<script src="./a11y-dialog.js"></script>
<script src="./a11y-dialog.cjs"></script>
</body>

</html>
14 changes: 14 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@
"main": "dist/a11y-dialog.js",
"module": "dist/a11y-dialog.esm.js",
"types": "dist/a11y-dialog.d.ts",
"exports": {
"import": "./dist/a11y-dialog.esm.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this is actually incorrect. The default exports needs to be under ".", which is a special reserved character for the default. In other words, we need to rename exports.import, exports.require, and exports.types to exports['.'].import, exports['.'].require, and exports['.'].types respectively.

Suggested change
"import": "./dist/a11y-dialog.esm.js",
".": {
"import": "./dist/a11y-dialog.esm.js",
"require": "./dist/a11y-dialog.cjs",
"types": "./dist/a11y-dialog.d.ts",
}

Copy link
Owner

Choose a reason for hiding this comment

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

As I mentioned in one of my messages, it fails if we use both "." and defined paths:

Error [ERR_INVALID_PACKAGE_CONFIG]: Invalid package config /Users/kitty/Sites/a11y-dialog-bschlenk-test/node_modules/a11y-dialog/package.json. "exports" cannot contain some keys starting with '.' and some not. The exports object must either be an object of package subpath keys or an object of main entry condition name keys only.

"require": "./dist/a11y-dialog.cjs",
"types": "./dist/a11y-dialog.d.ts",
"dist/*.esm.js": "./dist/*.esm.js",
"dist/*.cjs": "./dist/*.cjs",
"dist/*.js": "./dist/*.cjs",
"dist/*": {
"import": "./dist/*.esm.js",
"require": "./dist/*.cjs",
"types": "./dist/*.d.ts"
},
"package.json": "./package.json"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

general best practice to allow importing your package.json

},
"keywords": [
"modal",
"dialog",
Expand Down
6 changes: 3 additions & 3 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ export default [
output: [
{
...umdCfg,
file: 'dist/a11y-dialog.js',
file: 'dist/a11y-dialog.cjs',
},
{
...umdCfg,
file: 'dist/a11y-dialog.min.js',
file: 'dist/a11y-dialog.min.cjs',
plugins: [minify],
},
{
...umdCfg,
file: 'cypress/fixtures/a11y-dialog.js',
file: 'cypress/fixtures/a11y-dialog.cjs',
},
],
},
Expand Down