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

[Feature] Update Node.js version from v12 to v20 as GitHub Actions will no longer support v12 #626

Closed
wants to merge 4 commits into from
Closed

Conversation

danielmarcano
Copy link

@danielmarcano danielmarcano commented Oct 20, 2023

Related to Issue #9895

The following changes have been made because GitHub Actions will no longer support Node.js v12, as mentioned by @williamjallen in the linked issue. You can find the relevant information in the blog post referenced in that issue.

  1. Updated publish.yml so that it uses Node.js version 20
  2. Updated test.yml so that it uses Node.js version [20.x]
  3. Updated version to 2.5.2 in package.json to reflect this change

Checks:

  • Have run npm run lint and npm run test as specified in the README.md

P.S. If this PR is accepted, could you please add the "hacktoberfest-accepted" label to it, so that it counts towards my Hacktoberfest contributions?

…ws, as GitHub Actions will no longer support v12
package.json Outdated Show resolved Hide resolved
… Karma, so that they can work with Node.js v20
@danielmarcano
Copy link
Author

danielmarcano commented Oct 21, 2023

Hi, @williamjallen, I have figured out that the issue was using Webpack v4, as it does not support Node.js v20.

Thus, I have updated all Webpack, Webpack Plugins, and Karma packages to their latest versions so that they can support Node.js v20.

I have also updated Webpack and Karma configs to meet new Webpack v5 requirements and syntax.

Nonetheless, there's a small issue. Two tests are failing:

image

I have noticed that there's one test which, when skipped, allows these failing tests to pass, which means that somehow this test is leaking. However, I have not been able to find out how to fix it:

it.skip('should edit text annotation when overlay moved', function(done) {
    enableEdit();
    svg.appendChild(text);
    simulateMoveOverlay(function(args) {
      equal(editAnnotationSpy.called, true);
      equal(args[0], 'edit-document-id');
      equal(args[1], text.getAttribute('data-pdf-annotate-id'));
      equal(args[2], DEFAULT_TEXT_ANNOTATION);
      done();
    });
  });

Do you know what the issue might be?

@williamjallen
Copy link
Member

Does adding a disableEdit() at the end help? It looks a little bit odd to have timeouts all over the place as well, but cleaning up all of that isn't necessarily something we should fix in this PR.

I'm not particularly knowledgeable about this codebase (or Webpack in general for that matter). If you post in one of the Submitty Slack channels, someone who has worked with this repository might be able to provide more assistance.

@danielmarcano
Copy link
Author

danielmarcano commented Oct 21, 2023

disableEdit

Thank you, @williamjallen! I tried your suggestion, but it did not work. For now, I have skipped that sole test, and left a comment explaining why.

If you think we would be good to go, please take a look when you can and approve it if you think everything's okay.

@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

Merging #626 (bd3abb9) into main (14964a1) will decrease coverage by 0.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #626      +/-   ##
==========================================
- Coverage   73.77%   73.59%   -0.18%     
==========================================
  Files          38       38              
  Lines        1323     1333      +10     
==========================================
+ Hits          976      981       +5     
- Misses        347      352       +5     

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