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

create-react-app@5 compatibility #122

Merged
merged 4 commits into from
Mar 15, 2022
Merged

create-react-app@5 compatibility #122

merged 4 commits into from
Mar 15, 2022

Conversation

davidyuk
Copy link
Member

@davidyuk davidyuk commented Feb 9, 2022

closes #123

"Can't resolve 'stream'" is because of bs58check package (depends on create-hash => cipher-base => requires stream)

@davidyuk davidyuk marked this pull request as ready for review February 9, 2022 07:36
@@ -12726,7 +12722,8 @@
"util-deprecate": {
"version": "1.0.2",
"resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz",
"integrity": "sha1-RQ1Nyfpw3nMnYvvS1KKJgUGaDM8="
"integrity": "sha1-RQ1Nyfpw3nMnYvvS1KKJgUGaDM8=",
"dev": true
Copy link
Member

Choose a reason for hiding this comment

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

what is dev: true in the lock file ?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I understand the util-deprecate was used by bs58check (not a dev dependency), and since it was removed, then I assume there is some other package that still depends on util-deprecate.

Copy link
Member

Choose a reason for hiding this comment

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

found it in the docs :)

dincho
dincho previously approved these changes Feb 9, 2022
@marc0olo
Copy link
Contributor

@davidyuk is this ready? can we merge? we need a new release for calldata-lib then, right?

@davidyuk
Copy link
Member Author

I just found that blakejs also needs a fix: dcposch/blakejs#19

package.json Outdated
@@ -29,9 +29,10 @@
"./tests/test.js": "./tests/browser/test.js"
},
"dependencies": {
"blakejs": "^1.1.1",
"bs58check": "^2.1.2",
"blakejs": "github:aeternity/blakejs",
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see a fixed release version here, also a semver compliance of this lib

Copy link
Contributor

Choose a reason for hiding this comment

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

on tuesday the 15.03. we will either take the updated version of the lib or release and publish our own fork on npm, see dcposch/blakejs#19 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

@dincho just in case the contributor there could create a specific tag for the library on github - would it be ok to rely on that or do we want to have it published on NPM? see discussion here: dcposch/blakejs#20

Copy link
Member

Choose a reason for hiding this comment

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

NPM releases have one very strong property: https://docs.npmjs.com/policies/unpublish
So yes, I insist on NPM release dependancy.

@marc0olo marc0olo requested a review from dincho March 15, 2022 12:46
@marc0olo marc0olo self-requested a review March 15, 2022 12:57
@marc0olo marc0olo merged commit c13c7e9 into master Mar 15, 2022
@marc0olo marc0olo deleted the cra-5-compat branch March 15, 2022 13:00
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.

Not compatible with create-react-app@5
3 participants