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.

Sign up for GitHub

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

chore: align expected node engine with volta #7998

Merged
merged 5 commits into from
Oct 24, 2023
Merged

chore: align expected node engine with volta #7998

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d0002f
chore: align expected node engine with volta
alisonailea Oct 12, 2023
707154c
chore: add required node engine to published calicte-components
alisonailea Oct 24, 2023
0944233
fix: remove node engines as unnecessary
alisonailea Oct 24, 2023
a6099b3
docs: specify using the volta version for development
alisonailea Oct 24, 2023
19ae91c
chore: add volta reference back to calcite-components
alisonailea Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
},
"license": "SEE LICENSE.md",
"engines": {
"node": ">=16.0.0"
"node": ">=18.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Should this be >=18.18.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

anything over 18 will work. I wanted to keep it flexible but I can update

},
"packageManager": "npm@10.1.0",
"volta": {
Expand Down
4 changes: 2 additions & 2 deletions packages/calcite-components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
"@stencil/state-tunnel": "1.0.1"
},
"license": "SEE LICENSE.md",
"volta": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm removing volta at this level because a calcite-components consumer will not have the context of the monorepo to reference the volta node version.

Copy link
Member

Choose a reason for hiding this comment

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

The volta version is not for the calcite consumer. It is for automatically using the correct version of Node during development. Can you please add it back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since we should try to reserve the package.json published in the node module to only information needed by the consumer I think we should still leave volta out of the monorepo sub-packages package.json

Copy link
Member

Choose a reason for hiding this comment

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

If Volta adds another place to specify the development versions of node/npm then we can switch to that. But for now package.json is the only place to store pinned versions. We need to keep the volta field in CC's package.json so everyone who uses Volta will use the correct version of node.

We are following the recommendation for monorepo setups in Volta's doc:
https://docs.volta.sh/advanced/workspaces

The volta field in our package.json has no impact on our consumers, even if they use Volta. They would need to pin a version on their end.

Can you add volta back here and then I'll approve the PR? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after another discussion with Ben over teams I'm adding volta back to calcite-components

"extends": "../../package.json"
"engines": {
"node": ">=16.0.0"
}
}