Transition package from CJS to ESM

christian-bromann commented 2 years ago

As part of the v8 effort we are currently migrating over to ESM as many packages will stop support CJS in the future. This means we have to transition all plugins to ESM as well, ideally with continuous support for CJS.

goosewobbler commented 2 years ago

@christian-bromann is there prior art on any of the other service plugins? Switching to ESM is pretty trivial, ensuring support of CJS at the same time is not.

I was considering this approach to support both: https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html

christian-bromann commented 2 years ago

I am currently working in the WebdriverIO repo on the same thing. Currently it only creates ESM files but I want to find a way to also have a CJS export. I think I can achieve this with having two TypeScript configs as mentioned in the article.

goosewobbler commented 2 years ago

webdriverio main repo using tsup for this:

https://github.com/webdriverio/webdriverio/commit/25cbb4d44bf6f1c3332d36cb096a3c066bbe2031 https://github.com/egoist/tsup

christian-bromann commented 2 years ago

@goosewobbler I tried tsup but there seems to be some issues I haven't figured out yet, so there are chances that I switch to a different bundler.

christian-bromann commented 2 years ago

just fyi: I have been successfully experimenting with unbuild as compiler tool to transform the ESM code into CJS. Current problem is how to deal with ESM only dependencies where dynamic imports don't work because the dependency is used in a non async context.

christian-bromann commented 2 years ago

Another fyi: there is no need to have services and reporter export a ESM and CJS version. ESM only will be completely fine. Also the ESM transition can be done even after the v8 release given that you can import CJS modules in ESM.

goosewobbler commented 2 years ago

So it's ok to have this service export ESM only?

christian-bromann commented 2 years ago

So it's ok to have this service export ESM only?

Yes. ESM modules can import CJS modules.

goosewobbler commented 2 years ago

Presumably switching to only ESM will mean that the service will be incompatible with WDIO < v8?

christian-bromann commented 2 years ago

Presumably switching to only ESM will mean that the service will be incompatible with WDIO < v8?

That is correct. We could implement dynamic import to enable support for ESM plugins in v7.x though.

goosewobbler commented 2 years ago

Looked into this again and prototyped it. Seems ESM still needs some time to bake in many areas of the ecosystem. Also would prefer to wait until ESM is no longer experimental in Node. Jest is the main problem for this repo, when they have implemented a clean way to do mocking for ESM amongst other things, it should be a lot easier. I will keep this ticket open and look at it again in time.

https://jestjs.io/docs/ecmascript-modules

christian-bromann commented 2 years ago

@goosewobbler there is no rush to move to ESM given that WebdriverIO v8 should support CJS services as well. I think it is more a formality because you start see dependencies supporting only ESM at some point which can raise security issues. That said, with all packages I moved to ESM with I also had to port unit tests over to vitest which has similar interfaces as Jest but it still requires some work.

christian-bromann commented 2 years ago

I now have a reference PR for moving a service to ESM, check out the wdio-chromedriver-service example. Some important hints when doing this transition:

all imports need to reference a file with a .js extension (even though you are in TypeScript), to ensure this I used the 'import/extensions': ['error', 'ignorePackages'] EsLint rule
we should require Node.js v16 - latest across the board

in your package.json export the module as follows:

  "main": "./build/cjs/index.js",
  "type": "module",
  "module": "./build/index.js",
  "exports": {
    ".": [
      {
        "import": "./build/index.js",
        "require": "./build/cjs/index.js"
      },
      "./build/cjs/index.js"
    ]
  },
  "types": "./build/index.d.ts",
  "typeScriptVersion": "3.8.3",
  "engines": {
    "node": "^16.13 || >=18"
  },

write a separate CJS implementation that basically imports the original and calls the same hooks (see example)
use vitest for unit tests rather than Jest which doesn't have support yet

Let me know if you have any questions. I have released an alpha version of v8 which you can use to test your service.

goosewobbler commented 1 year ago

Hi @christian-bromann, I decided to dedicate some time to unbitrotting this service. My initial updates including chromedriver downloading and ESM updates can be found here:

https://github.com/webdriverio-community/wdio-electron-service/tree/update-deps

The chromedriver downloading seems to work but I can't get my integration tests to work due to ESM errors. Going to look at moving the wdio-electron-service-example repo into this one by converting it to a monorepo.

christian-bromann commented 1 year ago

Going to look at moving the wdio-electron-service-example repo into this one by converting it to a monorepo.

Including the example makes sense but I wouldn't transform the repo into a monorepo. Maybe just add the example into an example folder and link the dependencies so that someone can run the example to validate their changes to the code base.

goosewobbler commented 1 year ago

I wouldn't transform the repo into a monorepo. Maybe just add the example into an example folder and link the dependencies so that someone can run the example to validate their changes to the code base.

Updated for this approach. I think it's close to working but getting a bunch of these errors. Tried a lot of different things mostly around trying to set the ts-node esm flag but it does nothing...

> wdio run ./wdio.conf.js || (cat ./wdio-logs/wdio-0-0.log && cat ./wdio-logs/wdio-chromedriver.log && exit 1)

Execution of 4 workers started at 2023-01-17T19:17:34.517Z

[0-1] RUNNING in chrome - file:///test/application.spec.ts
[0-2] RUNNING in chrome - file:///test/dom.spec.ts
[0-0] RUNNING in chrome - file:///test/api.spec.ts
[0-3] RUNNING in chrome - file:///test/interaction.spec.ts
[0-2]  Error:  Unable to load spec files quite likely because they rely on `browser` object that is not fully initialised.
`browser` object has only `capabilities` and some flags like `isMobile`.
Helper files that use other `browser` commands have to be moved to `before` hook.
Spec file(s): file:///Workspace/wdio-electron-service/example/test/dom.spec.ts
Error: /Workspace/wdio-electron-service/example/test/dom.spec.ts:2
import { setupBrowser } from '@testing-library/webdriverio';
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at Object.compileFunction (node:vm:360:18)
    at wrapSafe (node:internal/modules/cjs/loader:1094:15)
    at Module._compile (node:internal/modules/cjs/loader:1129:27)
    at Module.m._compile (/Workspace/wdio-electron-service/example/node_modules/.pnpm/ts-node@10.9.1_awa2wsr5thmg3i7jqycphctjfq/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1219:10)
    at Object.require.extensions.<computed> [as .ts] (Workspace/wdio-electron-service/example/node_modules/.pnpm/ts-node@10.9.1_awa2wsr5thmg3i7jqycphctjfq/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1043:32)
    at Function.Module._load (node:internal/modules/cjs/loader:878:12)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)

goosewobbler commented 1 year ago

Released with v4

webdriverio-community / wdio-electron-service

Transition package from CJS to ESM #35