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

Transition package from CJS to ESM #35

Closed
Tracked by #8306
christian-bromann opened this issue May 25, 2022 · 17 comments
Closed
Tracked by #8306

Transition package from CJS to ESM #35

christian-bromann opened this issue May 25, 2022 · 17 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@christian-bromann
Copy link
Contributor

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 goosewobbler added the enhancement New feature or request label Jun 22, 2022
@goosewobbler
Copy link
Member

@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
Copy link
Contributor Author

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
Copy link
Member

webdriverio main repo using tsup for this:

webdriverio/webdriverio@25cbb4d
https://github.com/egoist/tsup

@christian-bromann
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

christian-bromann commented Jul 7, 2022

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
Copy link
Member

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

@christian-bromann
Copy link
Contributor Author

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

Yes. ESM modules can import CJS modules.

@goosewobbler
Copy link
Member

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

@christian-bromann
Copy link
Contributor Author

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
Copy link
Member

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

@goosewobbler goosewobbler removed this from the v4 milestone Aug 11, 2022
@christian-bromann
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

goosewobbler commented Jan 17, 2023

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
Copy link
Member

Released with v4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants