preactjs / enzyme-adapter-preact-pure

Preact adapter for the Enzyme UI testing library
MIT License
67 stars 17 forks source link

Add option to use `preact-render-to-string` for the StringRenderer #222

Closed andrewiggins closed 1 year ago

andrewiggins commented 1 year ago

The React adapter's string renderer uses renderToStaticMarkup from react-dom/server allowing the string render to run in environments without a DOM. This PR adds an option to enable using preact-render-to-string for our string renderer to enable our string renderer to also run in environments without a DOM.

The tests for this renderer teardown the JSDOM environment set up for its tests to ensure the adapter with the new flag works in an environment without JSDOM.

robertknight commented 1 year ago

This PR adds an option to enable using preact-render-to-string for our string renderer to enable our string renderer to also run in environments with a DOM.

You mean "run in environments without a DOM", right?

andrewiggins commented 1 year ago

I couldn't find a way to dynamically and synchronously require preact-render-to-string in both ESM and CJS files. In CJS you can obviously just use require. In ESM, you can use import but that's async and Enzyme doesn't have any support for async APIs. There is createRequire from node:module but it requires the current file path which in ESM needs to use import.meta.url which Node fails to even load a CJS module if its text includes import.meta.url. Here is a sample of what I tried:

import { createRequire } from 'module';
function includePreactRenderToString() {
  let nodeRequire: NodeRequire;
  if (typeof require !== 'undefined') {
    nodeRequire = require;
  } else {
    // NodeJS fails to even load this file as CJS when the following line exists even though it won't be executed
    // @ts-ignore
    nodeRequire = createRequire(import.meta.url);
  }

  // TODO: print help error message if RTS is not found
  return nodeRequire('preact-render-to-string');
}
includePreactRenderToString();

So I changed the adapter option to take in the renderToString function to consumers can include preact-render-to-string as a dependency and import it using whatever module system they are using.

Let me know what you think!

robertknight commented 1 year ago

So I changed the adapter option to take in the renderToString function to consumers can include preact-render-to-string as a dependency and import it using whatever module system they are using.

I like this approach. Slightly more work for the consumer, but it avoids a lot of complications with optional dependencies.

Regarding CJS vs ESM, I am inclined to drop CJS support in the next one or two major releases, if it does not cause major inconvenience for current library users. Thoughts?