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

Wechaty v1.x API #125

Merged
merged 43 commits into from
May 11, 2022
Merged

Wechaty v1.x API #125

merged 43 commits into from
May 11, 2022

Conversation

huan
Copy link
Member

@huan huan commented Jan 18, 2022

@padlocal I have upgraded the code base with the ES Module & Wechaty v1.x, please review and let me know what you think.

Thank you very much!

Related to:

@huan huan requested a review from padlocal January 18, 2022 18:46
@huan huan changed the title ES Module & Wechaty v1.x API Wechaty v1.x API Feb 6, 2022
@huan
Copy link
Member Author

huan commented Feb 6, 2022

Have removed the ESM from this PR with the latest commits.

I hope we can upgrade to Wechaty v1 soon!

@leochen-g
Copy link

leochen-g commented Feb 22, 2022

我把esm分支拉到本地跑了一下,发现安装依赖并没有问题,action 跑的时候可能是超时导致的失败。不过安装完依赖在运行npm run test 的时候,出现了一下报错,似乎是wechaty-puppet/types的引用存在问题,按照正常情况应该引用的是esm里的types,但是在运行test的时候查找的是cjs中的types

  • node环境:16.13.0
  • Mac
nvm use v16
Now using node v16.13.0 (npm v8.1.0)
➜  puppet-padlocal git:(esm) npm i
npm WARN deprecated @types/[email protected]: This is a stub types definition. quick-lru provides its own type definitions, so you do not need this installed.
npm WARN deprecated @hapi/[email protected]: Moved to 'npm install @sideway/pinpoint'
npm WARN deprecated @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
npm WARN deprecated [email protected]: this library is no longer supported
npm WARN deprecated @hapi/[email protected]: Moved to 'npm install @sideway/formula'
npm WARN deprecated @hapi/[email protected]: Moved to 'npm install @sideway/address'
npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated @hapi/[email protected]: This version has been deprecated and is no longer supported or maintained
npm WARN deprecated [email protected]: Critical security vulnerability fixed in v0.21.1. For more information, see https://github.com/axios/axios/pull/3410
npm WARN deprecated @hapi/[email protected]: Switch to 'npm install joi'

added 1175 packages, and audited 1337 packages in 2m

7 packages are looking for funding
  run `npm fund` for details

13 vulnerabilities (8 moderate, 5 high)

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
➜  puppet-padlocal git:(esm) npm run test

> [email protected] test
> npm-run-all lint test:unit


> [email protected] lint
> npm-run-all lint:es lint:ts lint:md


> [email protected] lint:es
> eslint "src/**/*.ts" "tests/**/*.test.ts" --ignore-pattern tests/fixtures/

=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <4.5.0

YOUR TYPESCRIPT VERSION: 4.5.5

Please only submit bug reports when using the officially supported version.

=============

> [email protected] lint:ts
> tsc --isolatedModules --noEmit


> [email protected] lint:md
> markdownlint README.md


> [email protected] test:unit
> jest

WARNING: NODE_ENV value of 'test' did not match any deployment config file names.
WARNING: See https://github.com/lorenwest/node-config/wiki/Strict-Mode
WARNING: NODE_ENV value of 'test' did not match any deployment config file names.
WARNING: See https://github.com/lorenwest/node-config/wiki/Strict-Mode
 FAIL  tests/wechaty-api.test.ts
  ● Test suite failed to run

    Cannot find module 'wechaty-puppet/types' from 'node_modules/wechaty/dist/cjs/src/mods/mod.js'

    Require stack:
      node_modules/wechaty/dist/cjs/src/mods/mod.js
      tests/wechaty-api.test.ts

      at Resolver.resolveModule (node_modules/jest-resolve/build/resolver.js:324:11)
      at Object.<anonymous> (node_modules/wechaty/src/mods/mod.ts:39:1)

 FAIL  tests/wechaty-login.test.ts
  ● Test suite failed to run

    Cannot find module 'wechaty-puppet/types' from 'node_modules/wechaty/dist/cjs/src/mods/mod.js'

    Require stack:
      node_modules/wechaty/dist/cjs/src/mods/mod.js
      tests/wechaty-common.ts
      tests/wechaty-login.test.ts

      at Resolver.resolveModule (node_modules/jest-resolve/build/resolver.js:324:11)
      at Object.<anonymous> (node_modules/wechaty/src/mods/mod.ts:39:1)

 FAIL  tests/wechaty-push.test.ts
  ● Test suite failed to run

    Cannot find module 'wechaty-puppet/types' from 'node_modules/wechaty/dist/cjs/src/mods/mod.js'

    Require stack:
      node_modules/wechaty/dist/cjs/src/mods/mod.js
      tests/wechaty-push.test.ts

      at Resolver.resolveModule (node_modules/jest-resolve/build/resolver.js:324:11)
      at Object.<anonymous> (node_modules/wechaty/src/mods/mod.ts:39:1)


ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From tests/wechaty-api.test.ts.

      at node_modules/gifwrap/src/gifcodec.js:7:15

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From tests/wechaty-push.test.ts.

      at node_modules/gifwrap/src/gifcodec.js:7:15

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From tests/wechaty-login.test.ts.

      at node_modules/gifwrap/src/gifcodec.js:7:15
Test Suites: 3 failed, 3 total
Tests:       0 total
Snapshots:   0 total
Time:        5.557 s
Ran all test suites.
ERROR: "test:unit" exited with 1.

@huan
Copy link
Member Author

huan commented Feb 22, 2022

Hi @leochen-g

Thank you very much for taking look at this PR!

The error messages you have posted is very wired because we can require wechaty without any problem with CLI:

$ node
Welcome to Node.js v16.13.2.
Type ".help" for more information.
> require('wechaty-puppet/types')
{
  ContactGender: [Getter],
  Contact: [Getter],
  FriendshipScene: [Getter],
  Friendship: [Getter],
  Image: [Getter],
  Message: [Getter],
  Post: [Getter],
  Tap: [Getter],
  Sayable: [Getter],
  Payload: [Getter],
  Dirty: [Getter],
  ScanStatus: [Getter],
  YOU: [Getter],
  CHAT_EVENT_DICT: [Getter],
  PUPPET_EVENT_DICT: [Getter]
}

So I added another minimum unit test that loads the wechaty and found that it can work without any problem, with tap runner:

$ ./tests/wechaty-login.spec.ts 
TAP version 13
# Subtest: smoke testing
    ok 1 - bot has created
    1..1
ok 1 - smoke testing # time=6.318ms

1..1
# time=17.664ms

Tap source code:

#!/usr/bin/env -S node -r ts-node/register
import { test } from "tstest";
import * as WECHATY from "wechaty";
test("smoke testing", async t => {
const bot = WECHATY.WechatyBuilder.build();
t.ok(bot, "bot has created");
});

So I guess the problem might relate to the jest test runner?

Not sure about it but hope the above information could be helpful for the future investigating.

@leochen-g
Copy link

@huan 有点不太明白哈,就是现在1.x版本使用的应该是esm的导入方式引入包,但是我看报错,引入的wechaty是cjs的,这个是不是不太对,packages.json 里我看并没有type: 'module' ,其他puppet里我看是有这个配置的,会和这个有关系吗

@huan
Copy link
Member Author

huan commented Feb 22, 2022

现在1.x版本使用的应该是esm的导入方式引入包

This is partially correct.

The 1.x version of Wechaty is built with a Dual-ESM-CJS pattern, which means that when it is loaded by an ESM module then it will be an ESM module, otherwise when it is loaded by a CJS module then it will be a CJS module.

When the Wechaty is loading by the correct version of the wechaty-puppet-padlocal (which is a CJS module), then it will act as a CJS module and be loaded as CJS.

但是我看报错,引入的wechaty是cjs的,这个是不是不太对

This is expected.

According to my above explanation: when the CJS wechaty-puppet-padlocal load the wechaty, the wechaty will be in CJS mode.

packages.json 里我看并没有type: 'module'

If the package.json specifies a type: 'module', then it means that the current module is ESM.

wechaty-puppet-padlocal has no type: 'module' so it's a CJS

其他puppet里我看是有这个配置的

wechaty, wechaty-puppet, and other Wechaty ecosystem modules have 'type: 'module' by default, which means that they are ESM modules.

And we have enabled the Dual-ESM-CJS for them when they have been built, so they can be used either by ESM or CJS.

I hope my explanations help you understand the situation, please feel free to let me know if you have more questions.

@leochen-g
Copy link

好的 谢谢解惑 我再研究一下

@huan
Copy link
Member Author

huan commented Feb 22, 2022

@leochen-g You are welcome!

@whyour
Copy link

whyour commented May 11, 2022

any progress?

@padlocal padlocal merged commit 13fe8ba into main May 11, 2022
@padlocal padlocal deleted the esm branch May 11, 2022 16:17
@huan
Copy link
Member Author

huan commented May 11, 2022

Finally! Congratulations! 🎉

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

Successfully merging this pull request may close these issues.

4 participants