ESM export?

Issue

I am trying to try this awesome library in my project, but I just found that it cannot be imported as a ESM module from my test files.

I am using web-test-runner (https://modern-web.dev/docs/test-runner/writing-tests/js-tests/) to run my tests, and as you can see, ESM imports are necesary, but it doesn't support TS or require/AMD exports

Would be possible to build the project allowing to do imports from JS files, like it's in TS?

Thanks!

I have tried to build the project for esnext but I have some problems into some dependencies. If you want I can provide a sample project to debug, logs or what you need :)

@vlaraort

Hey @vlaraort 👋

The ask is straight-forward enough so should be reasonable to do.

Some thought needed as to the best approach as there are a few approaches, e.g. using .cjs and .mjs vs 2 package.json alternatives.

In the meantime you might be able to unblock yourself by using the likes of https://jspm.org/, https://esm.sh/, or https://jsr.io/

I don't think you need two package.json, only ship two builds in the same package, and use this approach

https://antfu.me/posts/publish-esm-and-cjs

I will check in the meantime the solutions you provide, thanks for the quick response and the effort of this library! I think it's a awesome solution for having regression testing on accessibility, and it's something really needed in this times to aware about a11y ;)

Yep that’s the first (and newer) approach where you ship different extensions and utilise the newish exports field 😊

Prior that existing, the only option was to publish two packages, one with type:module.

Not sure what the tradeoffs are of the two approaches, but suspect the more modern approach is the way to go.

@vlaraort

Hi @vlaraort 👋

I’ve hopefully now updated the package to dual support CJS and ESM. Please could you try with 0.18.0?

I tried, thanks!

I received the following error and it crashes, I am going to check that dependency to see if this can be fixed :)

In fact, here are some information regarding the same dependency, in the same library

A11yance/aria-query#48

I tried, thanks!

I received the following error and it crashes, I am going to check that dependency to see if this can be fixed :)

Ah apologies, I should have tested more thoroughly 🤦

I’ll take a look as to whether there is anything I can do in this package to work around the issue, otherwise I guess we ask the question again on the aria-query thread!

Looks like they don't really want to provide a ESM export because reasons

A11yance/aria-query#485

But, there is a fork that makes the ESM build, maybe we can try with this fork of aria-roles instead?

https://github.com/KonnorRogers/aria-query/tree/konnorrogers/publish-modern-build

I have been playing around with the dependencies, making overrides from aria-roles to aria-roles-esm, and it's not the only dependency that causes problems in a ESM build. I also hit the wall with lz-string, that doesn't like that is going to work, so looks like it would be more difficult than assumed.

The only solution that comes to my mind (and I don't like too much), is to create a webpack/rollup/whatever bundle with all the dependencies embeed, so they are also processed and converted to ESM code. WDYT?

See also testing-library/dom-testing-library#995 it looks like this has been raised before, but the issue was quite muddled and closed based on a change that doesn’t quite go the whole way.

Maybe worth raising an issue on Testing Library to see what appetite would be?

Alternatively there are some comments that suggest you can use a commonjs Rollup plugin within the web-test-runner config that might help compile for ESM?

In the meantime using the likes of https://esm.sh/@guidepup/virtual-screen-reader@0.19.0 as mentioned before might be the way to go for now.

See https://modern-web.dev/docs/dev-server/plugins/rollup/ for a guide on how to use the Rollup plugins within web test runner, specifically the commonjs one.

Away from desk so can’t test atm

Yeah, it’s already in the right key: setupFilesAfterEnv: ['<rootDir>/jest.setup.js']

Jest’s docs have had that right for ages.

@philsherry

Heya @philsherry

Bare with, have reproduced and think making ground.

TIL a testEnvironment of jsdom will automatically assume a package export of browser which doesn't seem unreasonable... except ESM can be executed in a browser, but not handled at all well in a Jest context, hence the error r.e. export.

REF: https://jestjs.io/docs/configuration#testenvironmentoptions-object

Going to try releasing a new version with some exports tweaks to get this workable, hopefully without additional config.

@philsherry

@philsherry can you try @guidepup/virtual-screen-reader@^0.25.0 and @guidepup/jest@^0.4.0 and see if it gets us any closer?

Mate, looking good. @guidepup/jest is firing nicely from the setup file now.

I’m seeing this, though, and feel free to take this up on the @guidepup/jest repo.

Property 'toMatchScreenReaderSnapshot' does not exist on type 'JestMatchers<HTMLElement>'.ts(2339)

Question about the inclusion of the file. I’m trying to have 2 × describe blocks for each page: 1 × jest, 1 × virtual-screen-reader. Prior to the change you just made, what you see below was the only way I could get it working. Am I missing something completely obvious; is there an easier way to include this? I think I’m at that stage of the day where I’d fail spelling my own name correctly.

function setupA11yPage() {
  document.body.innerHTML = `
<div>
  …
</div>
`;
}

describe('matchers', () => {
  beforeEach(() => {
    setupA11yPage();
  });

  afterEach(() => {
    document.body.innerHTML = ``;
  });

  // I’m writing `@guidepup/jest` tests here — see above, where errors are mentioned.
  describe('snapshot tests', () => {
    test('…', async () => {});
  });

  // I’m writing `@guidepup/virtual-screen-reader` tests here.
  describe('virtual screen reader tests', () => {
    // navigate by headings
    test('…', async () => {
      // I can’t async the `describe` block, so I have to put this inside each
      // `test` block I write — which seems less than ideal, or wrong (to me).
      const { virtual } = await import('@guidepup/virtual-screen-reader');
    });
  });
});

On the first point, let’s raise the issue on that repo and will take a look why the types aren’t propagating through.

On the latter, hopefully you should just be able use a single import statement at the top of the file:

import { virtual, Virtual } from "@guidepup/virtual-screen-reader";

function setupA11yPage() {
  document.body.innerHTML = `
<div>
  …
</div>
`;
}

describe('matchers', () => {
  beforeEach(() => {
    setupA11yPage();
  });

  afterEach(() => {
    document.body.innerHTML = ``;
  });

  describe('snapshot tests', () => {
    test('…', async () => {});
  });

  describe('virtual screen reader tests', () => {
    // navigate by headings
    test('…', async () => {
      // Use the easy start export
      await virtual.start({ … });
      // … rest of test

      // Or use the class constructor if you need multiple instances - e.g. sharding in parallel.
      const vsr = new Virtual();
      await vsr.start({ … });
      // … rest of test
    });
  });
});

Give package exports weren’t quite right before it may have prompted you async importing per test if was erroring?

Even after making those changes, it’s barking at me in one of two ways.

Happy for you to open new Issues and just tag me into them.

@philsherry

@philsherry are you able to share the following?

Are you using "type": "module" in your package.json or .mjs/.mts and .cjs/.cts extensions?
What are the values you are using for these keys in your tsconfig.json?
- target
- module
- moduleResolution
- esModuleInterop

The first error indicates that your project is configured to consider the file a CommonJS module. The use of the import with VSR will target the ESM version of the library which can't be used in a CommonJS context except as a dynamic import.

The second screenshot is how you import the bundled, minified, browser compatible ESM version of the library. This isn't something that would expect folks to use outside of a browser context.

Pending further info, would suggest your paths forward are one of:

Use a dynamic import at the beginning of your test files in place of a static import, and continue to use the CJS module setup as-is:
```
const { Virtual } = await import("@guidepup/virtual-screen-reader");
```
Update your package.json (via "type": "module") and/or the file extension (i.e. .mts) to indicate that this file is in fact ESM.
Update your tsconfig.json to use a "moduleResolution": "bundler" to allow module resolution via VSR's exports instead of through your package module type / extension.

Not using "type": "module" in our package.json or .mjs/.mts and .cjs/.cts extensions.
Those tsconfig.json values:

{
  "esModuleInterop": true,
  "module": "CommonJS",
  "moduleResolution": "node",
  "target": "es2020"
}

✅ Will try this.
❌ Cannot do this.
❌ Cannot do this. (Long story. TL;DR, our pipeline blows up.)

Let me know how goes!

Failing that there is the option for the package to publish subpath exports to work around this, in your case something like @guidepup/virtual-screen-reader/cjs

Will have a play my end with an example matching that setup and see if any other options.

Yeah, I get shouted at for having that at the top.

The current file is a CommonJS module and cannot use 'await' at the top level.ts(1309)

We inherited the project, which was already forked off a previous project. It’s not a great situation for the developers but I’m trying to improve things for the users.

I’ll look at providing an export for your setup.

TLA was only reasonably recent so perhaps overly hopeful with your tsconfig values 😂

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#top-level-await

@philsherry

@philsherry had another go fiddling with the package exports, can you try again with:

@guidepup/virtual-screen-reader@^0.26.0
@guidepup/jest@^0.5.0

Where hopefully can just use static import statements for both libraries at the top of the appropriate files.

E.g. see https://github.com/guidepup/jest/blob/main/examples/typescript-legacy as an example (though in your jest setup file will have the import "@guidepup/jest"; instead of a direct reference to the lib directory).

Updated. Now I’m getting this.

 FAIL  src/pages/__test__/AccessibilityPage.a11y.tsx
  ● Test suite failed to run

    src/pages/__test__/AccessibilityPage.a11y.tsx:4:25 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@guidepup/virtual-screen-reader")' call instead.
      To convert this file to an ECMAScript module, add the field `"type": "module"` to '…/package.json'.

    import { virtual } from '@guidepup/virtual-screen-reader';

FWIW, I’ve hated this project since I joined it.

Hmm, starting to get a bit stuck then… without being able to see more of your projects setup 😅

Perhaps worth comparing against the https://github.com/guidepup/jest/blob/main/examples/typescript-legacy example and seeing where things are differing and sharing?

Alternatively if can share as much of your tsconfig and jest config/setup as is permittable then can have a better shot of recreating.

—-

Update: I’m now not optimistic, but introduced some new tests which found some type issues and are now resolved for node16 tsconfigs in the latest patch - given you’re on node (aka node10) not expecting to resolve, but it is a fix for a similar error.

I ended up being off sick for a couple of days. Came back to find your update edited in, so I've just updated things and… my tests are… passing? 🫣

Note: Unless I add @guidepup/jest into the test file as well—despite it being referenced in the setup file—like so:

import '@guidepup/jest';
import { virtual } from '@guidepup/virtual-screen-reader';

…then I get those errors I mentioned:

Property 'toMatchScreenReaderSnapshot' does not exist on type 'JestMatchers<null>'.
Property 'toMatchScreenReaderInlineSnapshot' does not exist on type 'JestMatchers<null>'.

But all is well if I add it into each test file. Not sure if that will mean anything to you, but I'm still happy to be tagged into any Issues for that particular beast.

Many thanks for sticking with me through this. 🙏

Navigation Menu

Repository navigation

ESM export? #49

Issue

12 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ESM export? #49

Description

Issue

Activity

vlaraort commented on Feb 14, 2024

cmorten commented on Feb 14, 2024

vlaraort commented on Feb 14, 2024

cmorten commented on Feb 14, 2024

cmorten commented on Feb 17, 2024

vlaraort commented on Feb 20, 2024

vlaraort commented on Feb 20, 2024

cmorten commented on Feb 20, 2024

vlaraort commented on Feb 20, 2024

vlaraort commented on Feb 21, 2024

cmorten commented on Feb 21, 2024

cmorten commented on Feb 21, 2024

12 remaining items

philsherry commented on Jun 24, 2024

jlp-craigmorten commented on Jun 24, 2024

jlp-craigmorten commented on Jun 24, 2024

philsherry commented on Jun 24, 2024

cmorten commented on Jun 24, 2024

philsherry commented on Jun 25, 2024

jlp-craigmorten commented on Jun 26, 2024

philsherry commented on Jun 27, 2024

cmorten commented on Jun 27, 2024

philsherry commented on Jun 27, 2024

cmorten commented on Jun 27, 2024

jlp-craigmorten commented on Jun 27, 2024

philsherry commented on Jun 27, 2024

cmorten commented on Jun 27, 2024

philsherry commented on Jul 2, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions