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

fix: Use file path imports #318

Merged
merged 4 commits into from
Apr 21, 2023
Merged

fix: Use file path imports #318

merged 4 commits into from
Apr 21, 2023

Conversation

FrederikBolding
Copy link
Contributor

Motivation

The goal of this PR is to stop relying on package.json exports, as it may break older build tools such as Browserify.

Description

Uses explicit imports for all imports that were previously using exports aliases.

Steps to test or reproduce

@CLAassistant
Copy link

CLAassistant commented Apr 19, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@FrederikBolding FrederikBolding changed the title Use explicit imports fix: Use explicit imports Apr 19, 2023
@FrederikBolding FrederikBolding marked this pull request as ready for review April 19, 2023 10:11
@FrederikBolding FrederikBolding requested a review from a team as a code owner April 19, 2023 10:11
@FrederikBolding
Copy link
Contributor Author

@g11tech My bad, tests should work now.

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm, waiting for @wemeetagain to give a look, @acolytec3 / @holgerd77 if you also want to review

@acolytec3
Copy link
Contributor

I'll run our browser tests against it and see if we can remove our aliases in karma and let you know.

@paulmillr
Copy link

let's merge it please, it's a blocker for some apps

@wemeetagain wemeetagain changed the title fix: Use explicit imports fix: Use file path imports Apr 21, 2023
@wemeetagain wemeetagain merged commit f459e92 into ChainSafe:master Apr 21, 2023
@FrederikBolding FrederikBolding deleted the use-explicit-imports branch April 21, 2023 13:28
@acolytec3
Copy link
Contributor

I checked this against our current master branch and the module resolution issues we had in our browser tests are no longer an issue. Can't wait to update our code with this.

@g11tech
Copy link
Contributor

g11tech commented Apr 21, 2023

thanks @acolytec3

@paulmillr @FrederikBolding i have release the lib: #319, pls try the new versions

@FrederikBolding
Copy link
Contributor Author

@acolytec3 I have a branch ready for when the new version goes up.

@g11tech I don't see it published on NPM quite yet.

@wemeetagain
Copy link
Member

CI is still running, give it a few minutes

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.

6 participants