factorio-blueprint
factorio-blueprint copied to clipboard
Improving the published package
I was looking at using this library for a project I am working on, however the published package makes it kind of difficult.
Would you accept a pull request to update the build process?
Key areas of improvement:
- Expose ES6 modules
- Expose Typescript definition files
- Improving type definitions.
- Required dependencies as dependencies (eg Victor)
Also running the build in the CI system would be helpful especially if it had some sort of formatting (prettier/other) and linting (eslint/tslint ?) configuration.
Yes, I would accept that. I remember having to do something a little hacky to access the TypeScript definitions, so I can understand the annoyance. Unfortunately I don't think Victor has type definitions.
Victor is quite old, it does actually cause a few headaches.. eg doesn't expose a default on its module exports.
To get it to compile properly, I think you need to add a esModuleInterop into the tsconfig.json.
Someone has also made types for it, in @types/victor
I have made a start on a branch https://github.com/blacha/factorio-blueprint/tree/chore/adding-build-pipeline
I also don't really have any preferences on linting/formatting.. So if you have any strong opinions on tabs vs spaces, single vs double quotes I can change it.
I would say just don't change a lot (e.g. Keep spacing as 2, keep { on same line as functions/ifs, etc).
I'm not attached to Victor, but I'm not sure if it's the best idea to write our own implementation (since we want to do more than just store the value—we add/subtract from it, multiply, clone, etc). I've looked a little but I can't seem to find any alternatives. Not sure exactly what to do.
Awesome, My goal is not to change anything if i can avoid it, Just to make sure its standard across all the files so my editor doesn't keep auto-formatting things causing my commits to get too large.
By just adding a base prettier config, it doesn't really change very much, it is mostly just multi lining long statements, I was going to look at making the default line length longer to reduce the amount of changes,
Here is the base commit so far: https://github.com/blacha/factorio-blueprint/commit/1be1c58804f23f6c462503318baec75545897f20 This is a work in progress, I am looking to reduce the amount of changes.
Its mostly just removing excess commas, adding/removing ; and making long lines multiline. Most of the code that was multi lined could be adjusted to be back on one line.
Victor seems fine as a library, just needs a little bit of work to get it going in Typescript/nodejs land, I have a build working with nodejs compiled as commonjs modules. So might be able to give you a couple PRs tomorrow.
I will split them into a format/linting PR + the node build PR
Its mostly just removing excess commas
Trailing commas are a good feature that makes editing easier and reduces diff churn, and Prettier allows enabling them in its config, and it only has them off by default for compatibility reasons that aren't relevant here.
Trailing commas are a good feature that makes editing easier and reduces diff churn, and Prettier allows enabling them in its config, and it only has them off by default for compatibility reasons that aren't relevant here.
I am neither for or against trailing commas. Most of the codebase does not have trailing commas.
So to reduce the size of the PR I left them off.
So to reduce the size of the PR I left them off.
MR size is a legit point, but removing the existing trailing commas, referring to them as "excess", and setting up automation that enforces their removal isn't a neutral action.