Skip to content

Upgrade dependencies and modernize testing infrastructure#767

Closed
JustinBeckwith wants to merge 2 commits intoNaturalNode:masterfrom
JustinBeckwith:upgrade-deps-modernize-testing
Closed

Upgrade dependencies and modernize testing infrastructure#767
JustinBeckwith wants to merge 2 commits intoNaturalNode:masterfrom
JustinBeckwith:upgrade-deps-modernize-testing

Conversation

@JustinBeckwith
Copy link
Contributor

@JustinBeckwith JustinBeckwith commented Feb 7, 2026

Greetings folks, and thanks for the wonderful library! I was trying to chase down a deprecation warning in my own library, and it was a transitive dependency on http-server. Turns out ... you weren't using it!

While I was in here, figured I'd burn some tokens upgrading your dev infrastructure and getting rid of all those deprecation and security warnings :)

Feedback welcomed!

Summary

This PR upgrades major dev dependencies, modernizes the browser testing infrastructure, and achieves zero npm vulnerabilities.

Changes

Dependency Upgrades

  • webpack: 4 → 5 with proper Node.js polyfill configuration
  • jasmine: 3 → 6 with jasmine-core 6
  • TypeScript: → 5.9.3
  • @types/node: Updated to latest
  • gulp: 4 → 5 (then removed, see below)

Testing Infrastructure

  • Replaced gulp-based browser testing with modern jasmine-browser-runner
  • Configured webpack 5 fallback for Node.js core modules
  • Added browserify util polyfill for proper browser compatibility
  • Created spec/support/jasmine-browser.json configuration

Dependency Cleanup

Removed unused dependencies:

  • standard (redundant with eslint)
  • ts-standard (redundant with @typescript-eslint)
  • gulp and gulp-jasmine-browser (replaced with jasmine-browser-runner)
  • inherits (transitive dependency, not directly used)

Jasmine 6 Compatibility

Fixed duplicate test suite and spec names to comply with Jasmine 6's strict duplicate checking:

  • Updated suite names to include language/context identifiers
  • Added indices to forEach loops in parametric tests
  • Differentiated similar test cases

Security

  • Reduced npm vulnerabilities from 23 to 0 🎉

Test Results

  • ✅ All 868 specs passing in Node.js
  • ✅ All 868 specs passing in browser
  • ✅ ESLint passing with no issues
  • ✅ 0 vulnerabilities

Breaking Changes

None - all tests pass and the API remains unchanged.

🤖 Generated with Claude Code

- Upgrade webpack 4 -> 5 with proper Node.js polyfill configuration
- Upgrade jasmine 3 -> 6 and fix duplicate test names
- Upgrade TypeScript to 5.9.3
- Replace gulp-based browser testing with jasmine-browser-runner
- Remove unused dependencies: standard, ts-standard, gulp, gulp-jasmine-browser, inherits
- Configure webpack to use browserify util polyfill for browser compatibility
- Reduce npm vulnerabilities from 23 to 0
- All 868 specs passing in both Node.js and browser environments

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@Hugo-ter-Doest
Copy link
Collaborator

Thanks.

I have to say that, while you generated this PR using AI, I now have to check all changes by hand before I can give the go ahead for merging it.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 21784494255

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.06%) to 87.091%

Files with Coverage Reduction New Missed Lines %
lib/natural/brill_pos_tagger/lib/Corpus.js 1 71.11%
dist/cjs/spec/brill_pos_tagger_spec.js 3 79.63%
Totals Coverage Status
Change from base Build 19036436502: -0.06%
Covered Lines: 8945
Relevant Lines: 9919

💛 - Coveralls

- Fix template literal lint errors in test files (SentimentAnalyzer_spec, hamming_distance_spec)
- Enable type-aware linting by adding parserOptions.project to .eslintrc.json
- Add npm run fix script for easy auto-fixing of lint issues

This ensures local ESLint catches the same issues as CI's ts-standard linter.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@Hugo-ter-Doest
Copy link
Collaborator

Hugo-ter-Doest commented Feb 10, 2026

I am going to close this PR because it proposes all kinds of changes that have nothing to do with modernisation. This is caused by the fact that this is an automatically generated PR. Please refrain from making PR's with AI. It costs a lot of time to analyse the changes an AI proposes. Furthermore we need to take the modernisation of the testing and dependencies step by step to make sure everything keeps working.

@JustinBeckwith
Copy link
Contributor Author

Believe me, I can appreciate the impact AI generated PRs on open source projects. However ... this actually does have a fair bit to do with modernization. You're sitting on a ton of out of date dependencies at dev time that have numerous reported and verified vulnerabilities. Yes, I happened to use an agent to do this - but I reviewed every change it made, and stand by the changes I made.

I suspect if I submitted this PR with a human written description, and didn't outright disclose I used claude to write it - nothing in here would be problematic. Of course, this is your project, and y'all should run it how you see fit, but I'd encourage you to have a closer look 🤷

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.

3 participants