search

paceaux / selector-finder

Find a CSS selector on a public site
MIT License
23 stars 3 forks source link

Convert project to ESM #22

Closed jonkoops closed 1 year ago

jonkoops commented 1 year ago

Converts the code to use ECMAScript modules.

Closes #17

paceaux commented 1 year ago

Related to: https://github.com/paceaux/selector-finder/issues/17

paceaux commented 1 year ago

Challenges still to solve

  1. Using chalk now results in errors because it thinks it's in the browser. This is caused by the jest environment being set to puppeteer. So we have to temporarily unset the jest-environment because chalk, which is in the log class, which is everywhere is erroring
  2. tests with mocks are failing because we need to dynamically import mocks or... something
  3. we'll have to figure out how to set up the puppeteer tests again.
jonkoops commented 1 year ago

Using chalk now results in errors because it thinks it's in the browser. This is caused by the jest environment being set to puppeteer. So we have to temporarily unset the jest-environment because chalk, which is in the log class, which is everywhere is erroring

@paceaux I think I may have fixed this with a73cf0d65d48086e2ab98d023ca59e942364e440, which enables Puppeteer only for selector-finder.puppeteer.test.js, leaving the other tests in the default environment. See the Jest documentation for more information.

This also seems to speed up the bootstrapping of the other tests, probably because the default environment has less overhead.

paceaux commented 1 year ago

The only thing we have left to sort out is the unit tests that fail for site-crawler.js. And the reason they fail is because they aren't using the mocked version of Axios (they're actually making Ajax requests to a url and returning a live result instead of the mocked result).

paceaux commented 1 year ago

All unit tests are passing now (And some are even fixed)

Next step is to do an npm link locally and make sure that the CLI behavior isn't changed.

Once we know that's the case, I'll merge to develop, update the package to a 2.0.0, merge to master, and publish

jonkoops commented 1 year ago

@paceaux we might want to bump the engine field to some more recent version before shipping a 2.0. Currently Node.js 16 is EOL, so we could raise the minimum to 18.

jonkoops commented 1 year ago

Did a review of all the code we have now and I think this is ready to go.

paceaux commented 1 year ago

I ran (manually) some full integration tests and this worked the same way it did before. So, merging to dev.