search

jonathanpmartins / v-money3

Vue3 currency input/directive mask
MIT License
103 stars 27 forks source link

Package lock bump and test fixes #32

Closed sougiovn closed 3 years ago

sougiovn commented 3 years ago

Running npm install updated the package-lock.json

Replaced the .replaceAll calls in puppeteer.test.js to .replace calls. I don't know why, but Node kinda sucks sometimes and I can't find a stable node version that implements .replaceAll. This seems to be a common issue: https://stackoverflow.com/questions/65295584/jest-typeerror-replaceall-is-not-a-function

So I took the shortest path and replaced to .replace with regex in order to avoid future compatibility issues with new contributors ~like me~

jonathanpmartins commented 3 years ago

I'm sorry man, but I will not merge this pull request. The main reason is that it will probably create merge issues with commits that I'm pushing now.

Running npm install updated the package-lock.json

This was very strange at first, until I learned the problem and came with a new npm script called bump-lock. https://github.com/jonathanpmartins/v-money3/blob/5528107900476edc5d726f5cdeea2916a7c67141/package.json#L18 This will mirror the package.json version and solve the mismatch problem with the lock file on every release ship.

About the tests, I had a CodeQl scanning alert about .replace('something', 'another'). The changes to .replaceAll were made in this commit: https://github.com/jonathanpmartins/v-money3/commit/496d90ae89f42f3c0f0edc697d44e6c222847118. It solved the problem but I forgot to force the usage of node 16. I think .replaceAll is available in node 15+

Now tests are run using the node binary installed from npm. https://github.com/jonathanpmartins/v-money3/blob/5528107900476edc5d726f5cdeea2916a7c67141/package.json#L17 In the next 3.15.2 it will be available and working.

Thank you very much for reporting issues. Sorry again for not merging this pull request! Please keep using and testing the library. It keep getting better every time.

sougiovn commented 3 years ago

@jonathanpmartins no problem, you have a good reason for it