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

Explicitly don't use a polyfill for vm #996

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Explicitly don't use a polyfill for vm #996

merged 1 commit into from
Nov 7, 2024

Conversation

jdbocarsly
Copy link
Member

In our build, we are getting a warning about the fact that we don't have a polyfill for one of cypress's subdependencies, which uses the node.js vm function:

warning  in ./node_modules/asn1.js/lib/asn1/api.js

Module not found: Error: Can't resolve 'vm' in '/Users/josh/datalabvue/webapp/node_modules/asn1.js/lib/asn1'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "vm": require.resolve("vm-browserify") }'
	- install 'vm-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "vm": false }

I don't believe we really need the polyfill since since we haven't had one in for a while and the tests don't seem broken, so this PR just makes that explicit in vue.config.js

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.38%. Comparing base (3230bb2) to head (140c9ab).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #996   +/-   ##
=======================================
  Coverage   68.38%   68.38%           
=======================================
  Files          62       62           
  Lines        3941     3941           
=======================================
  Hits         2695     2695           
  Misses       1246     1246           

Copy link

cypress bot commented Nov 7, 2024

datalab    Run #2738

Run Properties:  status check passed Passed #2738  •  git commit d1d8635289 ℹ️: Merge 140c9ab750e8b37af59c69647501f25fef043d80 into 3230bb23fa9c9805ab56608a1717...
Project datalab
Branch Review jdb/vm-polyfill-fix
Run status status check passed Passed #2738
Run duration 06m 13s
Commit git commit d1d8635289 ℹ️: Merge 140c9ab750e8b37af59c69647501f25fef043d80 into 3230bb23fa9c9805ab56608a1717...
Committer Josh Bocarsly
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 405
View all changes introduced in this branch ↗︎

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

polyfills 🤷

@ml-evs ml-evs changed the title Explicitely don't use a polyfill for vm Explicitly don't use a polyfill for vm Nov 7, 2024
@ml-evs ml-evs added build For issues/PRs pertaining to the build or deployment of the package dependency_updates For issues/PRs that update the dependencies of the package javascript Pull requests that update Javascript code labels Nov 7, 2024
@ml-evs ml-evs merged commit d381512 into main Nov 7, 2024
17 checks passed
@ml-evs ml-evs deleted the jdb/vm-polyfill-fix branch November 7, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build For issues/PRs pertaining to the build or deployment of the package dependency_updates For issues/PRs that update the dependencies of the package javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants