Use .mjs extension for esm files by IanVS · Pull Request #453 · testing-library/jest-dom

IanVS · 2022-04-28T00:50:30Z

What:

This uses .mjs for esm files being exported from the package, since the package itself is not "type": "module". I also marked it as explicitly "type": "commonjs" for good measure.

Also, I added a step to clean out the dist directory before each build.

And combined the dist/esm and dist/cjs directories, since now the files use different extensions.

Why:

#437 (comment)

How:

Adjusted the rollup config

Checklist:

Documentation N/A
Tests N/A
Updated Type Definitions N/A
Ready to be merged

I've opened this against the next branch, so that an alpha/beta version can be released.

codecov · 2022-04-28T00:52:08Z

Codecov Report

Merging #453 (3edeee3) into next (719b35b) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              next      #453   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines          624       624           
  Branches       231       231           
=========================================
  Hits           624       624

Impacted Files	Coverage Δ
src/to-have-form-values.js	`100.00% <ø> (ø)`
src/to-have-value.js	`100.00% <ø> (ø)`
src/utils.js	`100.00% <ø> (ø)`
src/index.js	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

IanVS · 2022-05-20T01:51:35Z

Hi @gnapse, @sheremet-va, how's this look?

IanVS · 2022-08-11T12:47:51Z

Any feedback on this?

sheremet-va · 2022-08-11T14:09:41Z

rollup.config.js

-      index: 'src/index.js',
-      matchers: 'src/matchers.js',
-    },
+    input: 'src/index.js',


Maybe it's better to provide an array for an input? So rollup can share code between files, otherwise matchers are duplicated.

I'm not terribly familiar with rollup. How can I specify an array for input, when each one should have a specific different output?

There is an example in Vitest repo: https://github.com/vitest-dev/vitest/blob/main/packages/vitest/rollup.config.js

You will need two entries, something like this:

export default [ { input: entries, output: { dir: 'dist', entryFileNames: '[name].mjs', format: 'esm' } }, { input: entries, output: { dir: 'dist', entryFileNames: '[name].js', format: 'cjs' } } ]

sheremet-va

Rollup now produces .js file with ESM content alongside correct .mjs file 😞

This file should be generated, so you don't have double code for /index and /matchers entry points.

I've added two possible solutions.

sheremet-va · 2022-09-19T13:33:05Z

rollup.config.js

+    input: entries,
+    output: {
+      dir: 'dist',
+      entryFileNames: '[name].mjs',


You can add chunkFileNames: '[name]-[hash].mjs', so chunks will also have correct filepath.

You can also add preserveModules: true, then rollup will not generate "chunks", it will have the same file structure as your source code.

I guess the first solution is closest to how it was, so 😄

Good eye, thanks for catching that. I've taken your first suggestion, and mirrored it on the .js as well, just for consistency / to be explicit.

sheremet-va

I've tried this file, and it unfortunately fails with folder error :(

// test.mjs
import {toBeChecked} from './dist/matchers.mjs'

console.log(toBeChecked)

IanVS · 2022-09-19T14:03:40Z

It works for me if I add the --experimental-specifier-resolution=node flag.

node --experimental-specifier-resolution=node ./test.mjs

I don't work directly in node.js much, and espeically not with esm, but I do know I've needed to add this flag in the past. Is that not what you normally do?

sheremet-va · 2022-09-19T14:06:41Z

It works for me if I add the --experimental-specifier-resolution=node flag.
node --experimental-specifier-resolution=node ./test.mjs
I don't work directly in node.js much, and espeically not with esm, but I do know I've needed to add this flag in the past. Is that not what you normally do?

Well, it will be removed when Loader APIs will be finalized. I guess the problem is the other package that doesn't define exports (so, ecosystem problem), or jest-dom can import directly from index.js file.

This was previously handled as a file in the root of the project that redirected to the built index file. This change removes a layer of indirection by removing src/extend-expect, and using exports to point to the index file rather than using a real file in the project root.

IanVS · 2022-09-19T14:29:15Z

Boy, ESM in node is kind of a PITA isn't it? lol.

I added extensions / index.js, and also handled a file that I guess I had missed previously, extend-expect.js. I think using exports like this will work, but I'm not sure if I need the extension in the export map, or not.

sheremet-va · 2022-09-19T14:35:27Z

Boy, ESM in node is kind of a PITA isn't it? lol.

Yes, ESM<->CJS is a crime against humanity 😢

I think using exports like this will work, but I'm not sure if I need the extension in the export map, or not.

If you want people to import @testing-library/dom/extend-expect, it will not work 👀

sheremet-va · 2022-09-19T14:37:15Z

I see that everything else is working fine. But I thought you should know:

import '@testing-library/jest-dom/extend-expect' // doesn't work
import '@testing-library/jest-dom/extend-expect.js' // works
const def = require('@testing-library/jest-dom/extend-expect') // doesn't work

sheremet-va · 2022-09-19T14:40:57Z

I think it might be a good idea to add a test for simply importing jest-dom 😄

IanVS · 2022-09-19T14:41:43Z

@gnapse @nickmccurdy what is the purpose of /extend-expect.js? If we're talking about making breaking changes for the next major anyway, is there a reason to keep this extra export around, that seems to have just re-exported the main index file anyway?

nickserv · 2022-09-21T07:02:30Z

I assume it's just there for back compatibility. I agree that we probably don't need to keep it around for the next major (as long as we move the one-line implementation back to index.js).

IanVS · 2022-09-22T00:19:18Z

@nickmccurdy, done, thanks. I believe this is ready for review.

sheremet-va

LGTM 👍🏻

IanVS · 2022-10-06T14:39:55Z

@nickmccurdy @gnapse Hi, what are your thoughts on merging this and cutting a beta soon?

IanVS · 2022-10-26T15:41:59Z

Hi, friendly ping that I think this is ready to go. :)

IanVS · 2023-08-22T12:43:23Z

Hi, it's a bit of a shame that a breaking change was released without this PR and the related change (#438) to build and publish ESM. Was there a reason not to include them? Is another breaking change on the horizon? @nickmccurdy @gnapse @kentcdodds. 🙏

kentcdodds · 2023-08-22T13:20:43Z

Sorry, there's a skeleton crew maintaining this project. That's why this wasn't merged. I'm afraid I only have the bandwidth to work on things that directly impact my own work.

IanVS · 2023-08-22T13:24:31Z

OK, thanks for responding. I only pinged you because I saw you were involved recently in the latest breaking release, and I'm a bit surprised that you're not using jest-dom and/or esm in your latest work. At any rate, I'm creating a new PR on top of the latest changes in main, to hopefully make it easier for current maintainers to review/merge.

kentcdodds · 2023-08-22T14:33:27Z

I am actually, but with vitest and I don't think I've had issues. I'm going to look at it again in the future and if this fixes things for me then I may merge your other PR unless another maintainer steps up before I get to it

jgoz · 2023-08-22T14:43:48Z

@IanVS I think dual-publishing ESM/CJS could (and should?) be done in a non-breaking way, especially now that we've already done one breaking change that is creating some confusion amongst users (😅).

IanVS · 2023-08-22T15:24:09Z

Thanks, @jgoz I agree after taking another look at it. This PR was removing extend-expect, but you already did that in #511. 🙌

I've opened #519 as another attempt to dual-publish, with the latest 6.0 changes you made.

nickserv · 2023-08-22T15:40:33Z

Shouldn't this be reopened so #519 can close it? I'm not seeing .mjs files in Jest DOM 6.

IanVS · 2023-08-22T15:54:03Z

@nickmccurdy This isn't an issue, it's a PR that I've decided to abandon and re-implement in #519. It can be re-opened if you'd like, but I don't think it should be reviewed/merged, so I was trying to keep things clean.

nickserv · 2023-08-22T15:59:55Z

You're right, I got mixed up when reviewing other notifications here.

Use .mjs extension for esm files

ae55360

sheremet-va suggested changes Aug 11, 2022

View reviewed changes

IanVS mentioned this pull request Aug 31, 2022

feat: Drop support for Node 12 #379

Closed

2 tasks

Use array for input

93a63c0

IanVS requested a review from sheremet-va September 19, 2022 13:17

sheremet-va suggested changes Sep 19, 2022

View reviewed changes

Fix chunk names

9f02ec0

IanVS requested a review from sheremet-va September 19, 2022 13:48

sheremet-va suggested changes Sep 19, 2022

View reviewed changes

IanVS added 2 commits September 19, 2022 10:26

Add extensions for packages that need them in node ESM

bb1742f

IanVS requested a review from sheremet-va September 19, 2022 14:29

Remove extend-expect export

3edeee3

sheremet-va approved these changes Sep 22, 2022

View reviewed changes

IanVS mentioned this pull request Oct 31, 2022

Use .mjs extension for esm files storybookjs/jest-dom#1

Merged

IanVS closed this Aug 22, 2023

IanVS mentioned this pull request Aug 22, 2023

Publish ESM and CJS #519

Merged

4 tasks

Conversation

IanVS commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

IanVS commented May 20, 2022

Uh oh!

IanVS commented Aug 11, 2022

Uh oh!

sheremet-va Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

IanVS Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

sheremet-va Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sheremet-va left a comment

Choose a reason for hiding this comment

Uh oh!

sheremet-va Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

IanVS Sep 19, 2022

Choose a reason for hiding this comment

Uh oh!

sheremet-va left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IanVS commented Sep 19, 2022

Uh oh!

sheremet-va commented Sep 19, 2022

Uh oh!

IanVS commented Sep 19, 2022

Uh oh!

sheremet-va commented Sep 19, 2022

Uh oh!

sheremet-va commented Sep 19, 2022

Uh oh!

sheremet-va commented Sep 19, 2022

Uh oh!

IanVS commented Sep 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nickserv commented Sep 21, 2022

Uh oh!

IanVS commented Sep 22, 2022

Uh oh!

sheremet-va left a comment

Choose a reason for hiding this comment

Uh oh!

IanVS commented Oct 6, 2022

Uh oh!

IanVS commented Oct 26, 2022

Uh oh!

IanVS commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kentcdodds commented Aug 22, 2023

Uh oh!

IanVS commented Aug 22, 2023

Uh oh!

kentcdodds commented Aug 22, 2023

Uh oh!

jgoz commented Aug 22, 2023

Uh oh!

IanVS commented Aug 22, 2023

Uh oh!

nickserv commented Aug 22, 2023

Uh oh!

IanVS commented Aug 22, 2023

Uh oh!

nickserv commented Aug 22, 2023

Uh oh!

IanVS commented Apr 28, 2022 •

edited

Loading

codecov bot commented Apr 28, 2022 •

edited

Loading

sheremet-va Aug 11, 2022 •

edited

Loading

sheremet-va left a comment •

edited

Loading

IanVS commented Sep 19, 2022 •

edited

Loading

IanVS commented Aug 22, 2023 •

edited

Loading