slevithan / xregexp

Extended JavaScript regular expressions
http://xregexp.com/
MIT License
3.31k stars 278 forks source link

npm release 5.0.0 is broken? #322

Closed s-weigand closed 3 years ago

s-weigand commented 3 years ago

First of all thanks for the awesome tool πŸ˜„

Today I got a PR from dependabot to update xregexp from 4.4.1 to 5.0.0 and my tests failed with the following message:

  ● Test suite failed to run

    Cannot find module '../../tools/output/blocks' from 'unicode-blocks.js'

    Require stack:
      node_modules/xregexp/lib/addons/unicode-blocks.js
      node_modules/xregexp/lib/index.js
      source/lib/parsers.ts
      tests/lib/parsers.test.ts

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:299:11)
      at Object.<anonymous> (node_modules/xregexp/lib/addons/unicode-blocks.js:13:38)

While investigating the error I found that the npm release tarball appears to contain an older built version where lib/addons/unicode-blocks.js still exists, but since the tools (tools/output/blocks.js and tools/scripts/block-regex.js) were removed in 4860122362c9822f35ab7f2deea7973a5815fcac it crashes. Also lib/addons/build.js says XRegExp.build 4.4.1, so my guess is that there was a hiccup with the new build before publishing.

slevithan commented 3 years ago

@s-weigand thanks for reporting! Hmm, unicode-blocks.js does not appear in any build/publish actions, but a local (old) copy appeared in lib/addons for me and presumably @josephfrazier as well when he published v5.0.0. I deleted it prior to publishing v5.0.1, just now. Can you update to v5.0.1 and verify whether this is resolved for you?

Also lib/addons/build.js says XRegExp.build 4.4.1, so my guess is that there was a hiccup with the new build before publishing.

Looks like all the files in lib still had 4.4.1 headers! Not sure what went wrong when it was published.

I've verified that the 5.0.1 npm release tarball has the correct version numbers in the lib file headers.

josephfrazier commented 3 years ago

Thanks for the speedy fix, @slevithan, and @s-weigand for reporting!

a local (old) copy appeared in lib/addons for me and presumably @josephfrazier as well when he published v5.0.0

Oops, I guess I should get in the habit of running git clean -fxd before publishing. Another option would be to explicitly enumerate all of the files to be published in the package.json files field, but that seems like overkill.

Looks like all the files in lib still had 4.4.1 headers! Not sure what went wrong when it was published.

Yikes, that's surprising to me. The prepublish script should be rebuilding all the code, so I'm not sure why it appeared not to have... pretty sure I remember seeing the build run when I typed npm publish:

    "babel": "babel src -d lib",
    "build-unicode-data": "node tools/scripts/category-regex.js && node tools/scripts/property-regex.js && node tools/scripts/script-regex.js",
    "prebuild": "npm run build-unicode-data && npm run lint && npm run babel",
    "build": "browserify lib/index.js --standalone XRegExp > xregexp-all.js",
    "pretest": "npm run build",
    "test": "nyc --reporter=lcov --reporter=text-summary jasmine JASMINE_CONFIG_PATH=tests/jasmine.json",
    "test-saucelabs": "npm run pretest && zuul tests/spec/*.js",
    "test-browser": "npm run test-saucelabs -- --local --open",
    "prepublish": "npm test"
s-weigand commented 3 years ago

Thanks a lot it works now πŸš€

Btw I can recommend rimraf in build scripts, as a way to ensure clean builds. Also having the CI build and publish the releases after tests passed, helps to make the release process as clean and easy as possible. If you are interested in a github-action workflow to do the publishing for you I could make a PR next week πŸ˜„

josephfrazier commented 3 years ago

If you are interested in a github-action workflow to do the publishing for you I could make a PR next week πŸ˜„

Ooh, that would be amazing! I suppose we'd have to put an NPM credential into github, but that shouldn't be a problem (maybe NPM can generate a new one for this particular purpose?)

slevithan commented 3 years ago

That sounds great!

Closing this since it's resolved.

Heads up that I've deprecated v5.0.0 on npm using npm deprecate xregexp@"5.0.0" "Broken release fixed in v5.0.1" --otp=123456