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

refactor(UBA): sideload HRE environment variable #289

Conversation

james-a-morris
Copy link
Contributor

This change essentially side-loads the HRE variables to prevent the need to directly interface with the Hardhat Runtime Environment.

@linear
Copy link

linear bot commented May 26, 2023

@james-a-morris james-a-morris force-pushed the james/acx-1166-resolve-scraper-hardhat-error-on-initialization branch from b2c3c8d to f765362 Compare May 26, 2023 20:10
@james-a-morris james-a-morris requested a review from mrice32 May 26, 2023 20:11
Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

LGTM! Great fix, @james-a-morris!

Two other concerns that are pre-existing, but I noticed when reading through this PR (Note: I don't think we need to worry about these now or in this PR, but they might come up in the future):

  1. We're exporting test utils (things that use chai, etc). This is a bit weird, and probably means we have to include test libraries like chai or hardhat in our regular dependencies and not dev dependencies. That won't break anything, but it increases the size of the exported package and is pretty nonstandard for a package meant to be used in production applications.
  2. In some of these utilities, we're importing nodejs-specific packages that will break if imported into a FE. These include path, and fs. I'm not sure if this is problematic today, but might end up causing breakages in the future.

I think this is good to go as is and we can worry about those others in the future if/when we need to!

This change essentially side-loads the HRE variables to prevent the need to directly interface with the Hardhat Runtime Environment.

Signed-off-by: james-a-morris <jaamorris@cs.stonybrook.edu>
Signed-off-by: james-a-morris <jaamorris@cs.stonybrook.edu>
@james-a-morris james-a-morris force-pushed the james/acx-1166-resolve-scraper-hardhat-error-on-initialization branch from 25a0089 to fe3dde2 Compare May 30, 2023 13:49
@james-a-morris james-a-morris merged commit b83d01c into master May 30, 2023
@james-a-morris james-a-morris deleted the james/acx-1166-resolve-scraper-hardhat-error-on-initialization branch May 30, 2023 13:59
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