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

js: Add top-level package.json and turbo build #5819

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

joncinque
Copy link
Contributor

Problem

There's no top-level package.json, which means that dependabot doesn't know how to handle the sub-packages in the SPL repo.

Solution

Add a top-level package.json! This follows a very similar model to https://github.com/solana-labs/solana-web3.js/blob/master/package.json, and also adds turbo for building, which means that we don't need to declare the dependencies by hand in the CI scripts, which is awesome.

It's still a little messy, but it should be much better to work with overall. The next step might be to isolate all of the JS testing and have them all build their required programs themselves, rather than waiting on the cargo-test-sbf step to build it for them. That should greatly reduce the CI time.

@joncinque joncinque requested a review from 2501babe November 13, 2023 22:08
@joncinque
Copy link
Contributor Author

Merging early to see how the dependabot run goes tomorrow morning, please feel free to still give feedback!

@joncinque joncinque merged commit 7a4af0a into solana-labs:master Nov 13, 2023
@joncinque joncinque deleted the toppackage branch November 13, 2023 23:43
Comment on lines -24 to +29
"lint": "yarn pretty && eslint --max-warnings 0 'src/*.ts'",
"lint:fix": "yarn pretty:fix && eslint 'src/*.ts' --fix",
"lint": "npm run pretty && eslint --max-warnings 0 'src/*.ts'",
"lint:fix": "npm run pretty:fix && eslint 'src/*.ts' --fix",
"pretty": "prettier --check '{src/*.ts,test/*/*.ts}'",
"pretty:fix": "prettier --write '{src/*.ts,test/*/*.ts}'",
"doc": "yarn typedoc src/index.ts",
"test": "yarn test:unit && yarn test:e2e",
"doc": "npm run typedoc src/index.ts",
"test": "npm run test:unit && npm run test:e2e",
Copy link
Contributor

Choose a reason for hiding this comment

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

why the change to npm instead of either pnpm or leaving it as-is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pnpm fails if yarn is mentioned in the scripts, but works with npm. Since the other packages use npm when calling other scripts, I decided for consistency. We could certainly change it to pnpm though!

@2501babe
Copy link
Contributor

turbo is surprisingly straightforward

@joncinque
Copy link
Contributor Author

Yeah it's really neat. The default build doesn't figure out inter-package dependencies within your workspace, and just tries to build everything all at once. Apparently turbo will parse everything out and build intelligently

@joncinque joncinque mentioned this pull request Nov 28, 2023
5 tasks
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.

2 participants