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

ESM/TypeScript: Cant get new 1.0.x version to work with Jest/Puppeteer/TypeScript or ESM #869

Closed
tmcconechy opened this issue Apr 7, 2022 · 23 comments
Labels
🐛 bug Something isn't working

Comments

@tmcconechy
Copy link

tmcconechy commented Apr 7, 2022

The problem

Not sure if this is a bug or support issue if it is is just let me know and i'll redirect my question to discussions or support.
The problem is i cant get your new 1.0.5 (any 1.0.0) version to work anymore in any configuration using jest/puppetteer.
Seems like all the tooling is not in place on those ends so not sure what to do.

Environment

  • Node version: 16.5.0
  • @percy/cli version: 1.0.5
  • Version of Percy SDK you’re using: 1.0.5
  • OS version: Mac OS Monterey
  • Type of shell command-line [interface]: zsh

Details

Given below details i cant seem to find a way to use the new version. So sort of at a road block and need to revert to 1.0.0-beta.76 which may not be sustainable if you change things. Also wondering if this should be a major version as it requires major project changes in order to use it

Comment on Versioning

Also (this took me a while to figure out...)
Reverting to "@percy/cli": "^1.0.0-beta.76" "@percy/puppeteer": "^2.0.0" works.

  • upgrading @percy/puppeteer from 2.0.0 to 2.0.1 -> gives you the new @percy/cli
  • upgrading @percy/cli from 1.0.0-beta.76 to 1.0.5 -> requires a lot of changes and the ESM and TS support with Jest is still a bit experimental and hard to get working (havent yet).

So given that shouldn't these be major versions with breaking changes and more details and examples? Cant seem to find an example of using puppetteer in TS or ESM with jest and percy.

Debug logs

  ● Ids Accordion Percy Tests › should not have visual regressions in new contrast theme (percy)
    Cannot find module '#logger' from 'node_modules/@percy/logger/dist/index.js'
      at Resolver.resolveModule (node_modules/jest-resolve/build/resolver.js:324:11)

Code to reproduce issue

Without ESM Conversion

With ESM Conversion

See the error Cannot find module '#logger' from 'node_modules/@percy/logger/dist/index.js'

@Robdel12
Copy link
Contributor

Robdel12 commented Apr 7, 2022

👋🏼 This sounds like there's some compilation of the SDK going on. I'll have to look at your build system when I get a moment but my guess is it's transforming / falling over at the dynamic import. The example app runs fine, so its likely the build process causing issues.

(FWIW, you'll need to use v1.x of the CLI with 2.0.1 of the Puppeteer SDK.)

@Robdel12 Robdel12 changed the title ESM: Cant get new 1.0.x version to work with Jest/Puppeteer ESM/TypeScript: Cant get new 1.0.x version to work with Jest/Puppeteer/TypeScript Apr 7, 2022
@tmcconechy
Copy link
Author

tmcconechy commented Apr 18, 2022

The "build process" as it may be is using Jest and TS (tsc) to load the files (we build with TS/Webpack) but not for running of the tests. What is the dynamic import thing its not something i've seen with the # on it? Cant find anything about it...

Maybe i need to add something to TS so it understands these imports? https://github.com/percy/cli/blob/master/packages/logger/src/index.js#L1

@tmcconechy
Copy link
Author

tmcconechy commented Apr 27, 2022

@Robdel12 i circled back on this. Still cant find a solution.

I cant see any way to use percy anymore with ES6 or TS after the new version. If you take a look at https://github.com/percy/example-percy-puppeteer it might run fine but if you updated it to ES6 "type": "module" you might start hitting the same issues since your doing a require here still https://github.com/percy/example-percy-puppeteer/blob/master/tests/todomvc_spec.js#L4

I think what it would take to fix this is maybe not doing a dynamic import? Could you do it twice and import each file?

i did try this in MY package.json but then it gives zsh: segmentation fault / didnt work

  "imports": {
    "#logger": {
      "node": "node_modules/@percy/logger/dist/logger.js",
      "default": "node_modules/@percy/logger/dist/browser.js"
    }
  },

Best branch so far is https://github.com/infor-design/enterprise-wc/tree/change-jest-to-esm steps above.

Let me know if i should go through support or anything as we have to stay on the beta version and cant go to ES modules and we are paying monthly so not sure if that will ever stop working?

BTW after some period of testing it out will get this and have to restart the command line (Mac)

tmcconechy@USROMTMCCONEC01 enterprise-wc3 % JEST_PUPPETEER_CONFIG=jest-puppeteer.config.cjs node --experimental-vm-modules node_modules/jest/bin/jest.js -- accordion-percy
(node:17278) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

 RUNS  test/ids-accordion/ids-accordion-percy-test.ts
zsh: segmentation fault  JEST_PUPPETEER_CONFIG=jest-puppeteer.config.cjs node --experimental-vm-module

Thanks

@tmcconechy tmcconechy changed the title ESM/TypeScript: Cant get new 1.0.x version to work with Jest/Puppeteer/TypeScript ESM/TypeScript: Cant get new 1.0.x version to work with Jest/Puppeteer/TypeScript or Es modules Apr 27, 2022
@tmcconechy tmcconechy changed the title ESM/TypeScript: Cant get new 1.0.x version to work with Jest/Puppeteer/TypeScript or Es modules ESM/TypeScript: Cant get new 1.0.x version to work with Jest/Puppeteer/TypeScript or ESM Apr 27, 2022
@frixatrix
Copy link

@Robdel12 hi, any updates on this?

@frixatrix
Copy link

@Robdel12 Here you go with an example repo for this case:
https://github.com/frixatrix/percy-ts

Please check and might be you'll be able to tell what's wrong in the build process?

@tmcconechy
Copy link
Author

tmcconechy commented May 2, 2022

@frixatrix yes we definitely have the same problem

I was trying to see if i could make something like this work, but was unable. So floating the idea in case you can?

{
  "compilerOptions": {
    "paths": {
      "#logger": ["../path/to/percy-logger/*"],
    }
  }
}

Im thinking it would be more compatible if the percy team avoided the import like this... Cant see any way to make this work anymore.

@frixatrix
Copy link

frixatrix commented May 2, 2022

@tmcconechy I found that if you change import in node_modules/@percy/logger/dist/logger.js

from

import Logger from '#logger';

to

import Logger from './logger.js';

It may not fail the test...

At least my example test passed after applying that.

PASS  tests/test-pass.ts
  Passing test
    ✓ should display "google" text on page (784 ms)

 PASS  tests/test-percy.ts
  Percy test
    ✓ should display "google" text on page (2129 ms)

And percy snapshot submission also sent out.

@tmcconechy
Copy link
Author

tmcconechy commented May 2, 2022

Yeah i agree i think maybe thats the easiest solution - not do these imports this way. Especially since the consumer dont know how to figure it out. But there is a case with two imports here https://github.com/percy/cli/blob/master/packages/logger/package.json#L31-L34 im not sure about.

So maybe you guys can just switch to normal imports? @Robdel12 ?

@frixatrix
Copy link

Would be great if anyone will fix that and let us know instead of one dropdead reply month ago and we as paying users will be able to stop using version which is:

[percy] Heads up! The current version of @percy/cli is more than 10 releases behind! 1.0.0-beta.76 -> 1.1.0

@tmcconechy
Copy link
Author

Agreed - did you try to "escalate" this with support? Thinking about it since no replies...

@Robdel12
Copy link
Contributor

Robdel12 commented May 2, 2022

👋🏼 Sorry! There's two of us to manage 16 SDKs and I also have to manage support in between that. We're currently working on reverting @percy/sdk-utils to be commonjs so build systems like TS or packages like jest-resolve won't try and recompile already compiled code. Hopefully will be this week.

@frixatrix
Copy link

Thank you @Robdel12 !

@tmcconechy Funny that I sent to support another issue (not related to TS and newer percy libraries, my current builds on "@percy/cli": "^1.0.0-beta.55", "@percy/puppeteer": "^2.0.0" had started failing without any changes in codebase) but they answered that the issue I sent is linked to this one and I should wait for fix on this.
Thank you for raising this weeks ago.

@Robdel12
Copy link
Contributor

Robdel12 commented May 5, 2022

👋🏼 This should be resolved with v1.1.2. I was able to confirm it's fixed for the reproduction that @frixatrix provided

build-complete

For @tmcconechy -- I was able to confirm the project builds and doesn't have resolve errors from jest-resolve. But after 30 or so mins of trying to get tests to run, I wasn't able to. It keeps hanging here for me. I believe it should work, but won't close this out until we know for sure.

image

@tmcconechy
Copy link
Author

tmcconechy commented May 6, 2022

@Robdel12 Thank You! If I'm reading the fix correctly i should'nt have to use ES modules right? Or is it still ES modules? First attempt im getting this:

  ● Ids Tag Percy Tests › should not have visual regressions in new contrast theme (percy)

    You need to run with a version of node that supports ES Modules in the VM API. See https://jestjs.io/docs/ecmascript-modules

      27 |       document.querySelector('ids-theme-switcher')?.setAttribute('mode', 'contrast');
      28 |     });
    > 29 |     await percySnapshot(page, 'ids-tag-new-contrast');
         |                        ^
      30 |   });
      31 | });
      32 |

      at invariant (node_modules/jest-runtime/build/index.js:2569:11)
      at percySnapshot (node_modules/@percy/puppeteer/index.js:9:15)
      at Object.<anonymous> (test/ids-tag/ids-tag-percy-test.ts:29:24)

@frixatrix
Copy link

frixatrix commented May 9, 2022

@Robdel12
Hi, thank you for the fix, probably it may helped the others but not me.

I prepared another example repo with tests which are failing if there is percySnapshot: https://github.com/frixatrix/new-percy-ts

This repo setup is kind of I have on my projects and I don't get how can I run bunch of existing Percy tests again.
Please help to get at least those become running and I hope I'll be able to manage another ones too.


And that's weird but in my project tests with percy does not fail - they just never finishes despite any timeouts for jest/jest-puppeter (might be similar to https://github.com/gliff-ai/manage/issues/334 )

UPD: Was able to reproduce forever hanging tests with percySnapshot on https://github.com/percy/example-percy-puppeteer repo with using jest-puppeteer instead of mocha (no typescript at all).
Please see: https://github.com/frixatrix/example-percy-jest-puppeteer

@tmcconechy
Copy link
Author

After further attempts im still stuck on the problem above. I can run "experimental VM" but when i do this i just get segmentation faults. It appears the fix you made @Robdel12 would remove ES modules so i dont get 100% why maybe there is some part left in and additional fix is needed?

I would suggest in a tool like this we go for ES Modules when its not blocked in Jest https://jestjs.io/blog/2022/04/25/jest-28#ecmascript-modules and not "experimental" in node js.

@frixatrix
Copy link

@tmcconechy there's even request about it supporting ESM: percy/percy-puppeteer#426

@Robdel12
Copy link
Contributor

Robdel12 commented May 9, 2022

and not "experimental" in node js

It's no longer experimental in Node (hasn't been since 2 years ago with Node 14). We're likely going to undo the dynamic import since Jest is so far behind (it looks close to abandonware with just one maintainer really doing all the work).

@Robdel12
Copy link
Contributor

Robdel12 commented May 9, 2022

@frixatrix Just confirmed backing @percy/puppeteer to v2.0.0 (no dynamic import) fixes things with the latest CLI release. We'll have PRs/releases today that removes the dynamic imports. But you should be able to get to the latest CLI version with the Puppeteer SDK on v2.0.0 (not v2.0.1). Just make sure the lockfile updates to the latest version of @percy/sdk-utils (which is what we fixed in #916)

@frixatrix
Copy link

frixatrix commented May 9, 2022

@Robdel12 Thank you!

Just've confirmed that
"@percy/cli": "1.1.2", "@percy/puppeteer": "2.0.0",
fixed my issue with existing tests!


Tomorrow will check if that also fixes hanging in https://github.com/frixatrix/example-percy-jest-puppeteer .

UPD on ^:
It works with
"@percy/cli": "1.1.3", "@percy/puppeteer": "2.0.2",

Updated repo.

@Robdel12
Copy link
Contributor

Robdel12 commented May 9, 2022

Puppeteer v2.0.2 was released and should remove the need to have the esm flag with Jest. https://github.com/percy/percy-puppeteer/releases/tag/v2.0.2 Hopefully this is the end of these issues for y'all! 😅

@tmcconechy
Copy link
Author

Thanks so much @Robdel12 - can confirm this solves it for me. Feel free to close (whatever your process is for that)

@Robdel12
Copy link
Contributor

Awesome! Super happy to hear 😃 I'll close this up now -- if anything creeps up feel free to comment again 💯

@Robdel12 Robdel12 added the 🐛 bug Something isn't working label May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants