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

Extend AngularJS filecontent regex to match contents of 1.8.0 AngularJS #426

Merged
merged 4 commits into from
Jan 10, 2024
Merged

Extend AngularJS filecontent regex to match contents of 1.8.0 AngularJS #426

merged 4 commits into from
Jan 10, 2024

Conversation

rossturner
Copy link
Contributor

retire.js is not currently recognising AngularJS 1.8.0 and potentially other versions.

The first filecontent regular expression is as follows: /\*[ \n]+AngularJS v(§§version§§)

This matches https://code.angularjs.org/1.8.0/angular.min.js because the comment at the top of the file is

/*
 AngularJS v1.8.0
 (c) 2010-2020 Google, Inc. http://angularjs.org
 License: MIT
*/

However, https://code.angularjs.org/1.8.0/angular.js has a header of

/**
 * @license AngularJS v1.8.0
 * (c) 2010-2020 Google, Inc. http://angularjs.org
 * License: MIT
 */

Which does not match against the current implementation of the regex. This PR adds an optional non-capturing group of "@license " as a prefix to cover this case.

@eoftedal
Copy link
Contributor

eoftedal commented Jan 9, 2024

Would you mind adding a test case to https://github.com/RetireJS/retire.js/blob/master/repository/testcases.json ?
You can run the tests for a specific library by running:
./test-detection.js angularjs

Basically you would add something like:

"angularjs": {
    "https://code.angularjs.org/§§version§§/angular§§subversion§§.js": {
      "versions": ["1.8.0"],
      "subversions": ["", ".min"]
    }
}

@eoftedal eoftedal self-assigned this Jan 9, 2024
@eoftedal
Copy link
Contributor

eoftedal commented Jan 9, 2024

I think your fix doesn't take into account the extra * on the second line.

@rossturner
Copy link
Contributor Author

Ah good points thanks, I'll look at those

@rossturner
Copy link
Contributor Author

@eoftedal Thanks for pointing out the tests that I'd missed, can confirm that the additional test cases fail for AngularJS >= 1.7.0 with the old implementation, and pass with the changes in this PR

@eoftedal eoftedal merged commit 2c1d744 into RetireJS:master Jan 10, 2024
3 checks passed
@eoftedal
Copy link
Contributor

Thank you for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants