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

Change imports from /dist to index.js in root for npm packages #101

Closed
Divide-By-0 opened this issue Sep 17, 2023 · 5 comments
Closed

Change imports from /dist to index.js in root for npm packages #101

Divide-By-0 opened this issue Sep 17, 2023 · 5 comments
Assignees
Labels
bug Something isn't working easy good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Divide-By-0
Copy link
Member

We need to cleanup the helpers package etc so these imports can come from the root!

You are importing from @zk-email/helpers/src which is the .ts code - to transpile ts code in node_modules we would need additional config

You can change the import to /dist - import { toCircomBigIntBytes } from "@zk-email/helpers/dist/binaryFormat";

@Divide-By-0 Divide-By-0 added bug Something isn't working easy labels Sep 17, 2023
@Divide-By-0 Divide-By-0 added help wanted Extra attention is needed good first issue Good for newcomers labels Nov 24, 2023
@Divide-By-0 Divide-By-0 assigned isidroamv and unassigned saleel Nov 24, 2023
@isidroamv
Copy link
Collaborator

I believe it would be ideal import the modules in the format:

import { toCircomBigIntBytes } from "@zk-email/helpers/binaryFormat"; instead of
import { toCircomBigIntBytes } from "@zk-email/helpers/dist/binaryFormat";

As long we export the functions from index.ts from the root and index.js from the dist the functions should be accesible from both.

@Divide-By-0

@isidroamv
Copy link
Collaborator

Or even better:

import { toCircomBigIntBytes } from "@zk-email/helpers";

@Divide-By-0
Copy link
Member Author

Or even better:

import { toCircomBigIntBytes } from "@zk-email/helpers";

Absolutely! Metachaser's original docs pr tried to do this but our tests were super broken then.

@Metachaser24
Copy link
Contributor

@isidroamv this is done right?

@isidroamv
Copy link
Collaborator

@isidroamv this is done right?

Yes, this was completed on the PR #153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working easy good first issue Good for newcomers help wanted Extra attention is needed
Projects
Status: Done
Development

No branches or pull requests

4 participants