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

Add the missing common/helpers.ts to package.json #623

Merged
merged 1 commit into from
May 18, 2022
Merged

Conversation

ekpyron
Copy link
Member

@ekpyron ekpyron commented May 17, 2022

Fixes #622.

I guess this is enough to fix the issue, but maybe better if @cameel and/or @stephensli confirmed.

@ekpyron ekpyron changed the title Add common files to package.json. Add common helper file to package.json. May 17, 2022
@stephen-slm
Copy link
Contributor

@cameel this will do it, please just do a npm build-tarball and check the tarball output to see if the content contains everything required. If anything else is missing (hopefully not) include it in this PR @ekpyron

@nataouze
Copy link

Looks like "common/types.js" should be added too as it is also imported in linker.ts

@ekpyron
Copy link
Member Author

ekpyron commented May 17, 2022

As far as I can see dist/common/types.js (i.e. the js file built from common/types.ts), is empty and not imported by any of the other built js files - probably since it's only relevant for typescript and does not carry over to js...

@nataouze
Copy link

Yes you are right, it is not present in the built linker.js file. Thank you

Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Looks fine to me but not approving yet since I'd like to play with it a bit tomorrow and also investigate why this does not trigger a failure in CI.

@stephen-slm
Copy link
Contributor

stephen-slm commented May 17, 2022

@cameel I would not invest much effort as #615 updates the exported files. Which can be seen here

solc-js/package.json

Lines 38 to 44 in 95ca828

"files": [
"common/*.d.ts",
"common/*.js",
"bindings/*.d.ts",
"bindings/*.js",
"*.d.ts",
"*.js"

@cameel
Copy link
Member

cameel commented May 18, 2022

Well, we still need to find out why it went through our tests unnoticed. And then, when we fix the tests, we should rebase all these PRs on top of that to make sure the fix still passes them. I hope that the missing entry in package.json is the only problem but we really can't be sure of anything until tests are working properly.

@cameel cameel changed the title Add common helper file to package.json. Add the missing common/helpers.ts to package.json May 18, 2022
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Looks like there were no other issues. Tests with my fix in #625 now detect this problem and they do pass when rebased on top of this PR. See #625 (comment).

@ekpyron ekpyron merged commit 4377232 into master May 18, 2022
@ekpyron ekpyron deleted the fixPackage branch May 18, 2022 17:38
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.

Uncaught Error: Cannot find module './common/helpers' on 0.8.14
5 participants