Suggestion for JS workspace improvements
Assume lerna stays.
Task: remove babel
Reason: not needed anymore
Task: delete package-lock.json from all packages keep only one at the js root folder.
Reason: It is not necessary to have lock file in each package when you have a workspace
Task: move all devDependencies from each package.json to the root package.json.
Reason: it makes maintains/updating/install much simpler and keep the infra unified
Task: avoid .sh scripts and convert them to .js
Reason: better cross platform (Windows) support
Task: move tsconfig to the js root and extends each package
Reason: unify the configuration between packages, also since the build does not uses typescript the tsconfig is only used by the IDE.
Task: add prettier Reason: enforce similar code format
let me know what if this sounds reasonable.
Hey @barak007 thank you for this! And I've looked through your PR for the same, I appreciate it. Some of these items I'm definitely on board with, some of them I'm not sure. Either way, I will definitely want to go in small steps here as we move forward; I think the current #28 tries to cover too many of these things at once for my preferences.
Let me respond to each task in particular–
Task: remove
babelReason: not needed anymore
Really? I'm excited to hear that! It was in there for something that jest needed to run the tests effectively, but perhaps that's no longer relevant. I would love to take a PR just to get babel out
Task: delete
package-lock.jsonfrom all packages keep only one at thejsroot folder. Reason: It is not necessary to have lock file in each package when you have a workspace
I get the idea, but why is workspaces better than lerna? This seems to me a bit like a "if it's not broken, don't fix it" situation. Lerna covers the same set of solutions, no?
Task: move all
devDependenciesfrom eachpackage.jsonto the rootpackage.json. Reason: it makes maintains/updating/install much simpler and keep the infra unified
Agree but at the same time I'm not sold: it also seems like it would make it harder for a given package to deviate from the configuration of the others if it needed to. There is a reason (unfortunately) that the different js packages use different build workflows, which makes me a bit hesitant to try to unify them too early
Task: avoid
.shscripts and convert them to.jsReason: better cross platform (Windows) support
Definitely! Good catch, I'd love to take a PR for this one as well
Task: move
tsconfigto thejsroot and extends each package Reason: unify the configuration between packages, also since the build does not uses typescript thetsconfigis only used by the IDE.
I'm interested in this change, and also a bit confused– the builds do use typescript. Or do you mean that the packages ultimately ship as js with .d.ts files?
Task: add prettier Reason: enforce similar code format
I'm on board! For this one I'd say let's start by aligning on a config, then I'd like to approach the actual change set with 1 commit including the config files and a second commit which actually runs prettier and commits the result (imo this makes it clear when trying to follow the git history exactly what happened; you wouldn't need to go unearthing the config file from a single commit that touched many files)
Let me know what you think!
Hey good to hear.
First you can see the preview for these changes in https://github.com/elemaudio/elementary/pull/28 and I will also response one by one.
Really? I'm excited to hear that! It was in there for something that jest needed to run the tests effectively, but perhaps that's no longer relevant. I would love to take a PR just to get babel out
This is a semi big change which raises some type questions. In order to remove babel I used ts-jest this is why I changed the test files to .ts in the PR, this is why I found some type issues that I had to fix and also possible bug. (from that the questions)
I get the idea, but why is workspaces better than lerna? This seems to me a bit like a "if it's not broken, don't fix it" situation. Lerna covers the same set of solutions, no?
From my experience if you have npm workspace, lerna become unnecessary dependency as it's only giving you a good version bump mechanism over the workspace features of npm.
The version bump is valuable but you can still remove lerna from the deps tree and use npx to run it once you ready to release. it will also shrink the install time for anyone how does not releasing code.
Agree but at the same time I'm not sold: it also seems like it would make it harder for a given package to deviate from the configuration of the others if it needed to. There is a reason (unfortunately) that the different js packages use different build workflows, which makes me a bit hesitant to try to unify them too early
The way I see it is you have your tools in one place and you use/configure them differently in each package. It's not this or that you can still have both for extreme cases (which you don't have currently). I mean that you can still install a "special" dev dependency on a specific package but I don't see a case for that. this change is not for unifying the build/behavior of packages, just organize them in one place.
Definitely! Good catch, I'd love to take a PR for this one as well
Nice
I'm interested in this change, and also a bit confused– the builds do use typescript. Or do you mean that the packages ultimately ship as js with .d.ts files?
Currently this project does not build with tsc it uses bundlers (rollup, tsup) and they handle the typescript from source. so the tsconfig is solely for the ide.
This does not block per package overrides.
- if there is a need for customizing the Typescript build config it will be done via the bundler config.
- override tsconfig can be done with "extends" of the base config.
I'm on board! For this one I'd say let's start by aligning on a config, then I'd like to approach the actual change set with 1 commit including the config files and a second commit which actually runs prettier and commits the result (imo this makes it clear when trying to follow the git history exactly what happened; you wouldn't need to go unearthing the config file from a single commit that touched many files)
Sure, this is my standard .prettierrc config for typescript. as you can see nothing special and the code look consistent. (I will not argue on any rule)
{
"printWidth": 120,
"singleQuote": true,
"overrides": [
{
"files": ["*.js", "*.cjs", "*.mjs", "*.ts", "*.tsx", "*.cts", "*.mts"],
"options": {
"tabWidth": 4
}
}
]
}