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

Support broader range of dependency versions #758

Draft
wants to merge 7 commits into
base: testing-min-dependencies
Choose a base branch
from
Draft

Support broader range of dependency versions #758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ebdcf4d
Fix the object-shorthand warnings that popped up after eslint update
cameel Jan 25, 2025
1833cf0
solcjs: Remove the redundant --version option, conflicting with the b…
cameel Jan 25, 2025
9bfa616
package.json: Accept older dependency versions in cases where the pac…
cameel Jan 25, 2025
591df88
package.json: Accept a wider range of versions for the `tmp` and `js-…
cameel Jan 25, 2025
156e4b0
package.json: Adjust dependencies to accept updates even across break…
cameel Jan 25, 2025
15f272b
package.json: Restrict commander to <13.0.0 until we're ready to upgrade
cameel Jan 25, 2025
284967f
package.json: Restrict versions of dependencies that fail on node v12…
cameel Jan 25, 2025
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 formatters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export function formatFatalError (message) {
type: 'JSONError',
component: 'solcjs',
severity: 'error',
message: message,
message,
formattedMessage: 'Error: ' + message
}
]
Expand Down
48 changes: 24 additions & 24 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,32 @@
},
"homepage": "https://github.com/ethereum/solc-js#readme",
"dependencies": {
"command-exists": "^1.2.8",
"commander": "^8.1.0",
"follow-redirects": "^1.12.1",
"js-sha3": "0.8.0",
"memorystream": "^0.3.1",
"semver": "^5.5.0",
"tmp": "0.0.33"
"command-exists": ">=1.1.0",
Copy link
Member Author

@cameel cameel Jan 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Note that I replaced ^ with >= in all version ranges. I'm still on the fence whether that's a good idea or whether I should always put a < there to keep major version updates manual:

Pros of >=:

  • We almost never update these versions and this forces projects using solc-js to use old versions too (or do hacky workarounds). >= forces the update on us by default and we can always restrict with < as needed.
  • There's not much risk of this seriously breaking a downstream app that's already using the package. If it happens, the app can always restrict the dependency in its own package.json. And that's only a concern for the devs of the app, not its users. It may only happen when devs update their dependencies - outside of that the installed versions are locked via package-lock.json.

Cons of >=:

  • Already released versions in npm will stay unlimited forever and eventually stop working with latest versions of dependencies because one of them will inevitably introduce an actual breaking change. This may confuse someone trying to add a dependency on an older version of solc-js to their app.
  • There's some risk of a new breaking version appearing during our release process. Though in solc-js this would be easy to fix quickly, because CI is quick and it gets tagged late in the process.

"commander": ">=8.0.0 <12.0.0",
Copy link
Member Author

@cameel cameel Jan 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Versions >=13.0.0 do not pass our tests. We could probably support them if we wanted but that will require some code changes on our side. Not sure how extensive.

Versions >=12.0.0 trigger failures on older node.js versions due to the use of ?? operator or its variants.

"follow-redirects": ">=1.0.0",
"js-sha3": ">=0.8.0",
"memorystream": ">=0.3.0",
"semver": ">=5.0.0",
"tmp": ">=0.0.26"
},
"devDependencies": {
"@types/node": "^16.11.7",
"@types/semver": "^7.3.9",
"@types/tape": "^4.13.2",
"@types/tmp": "^0.2.3",
"@typescript-eslint/eslint-plugin": "^5.8.0",
"@typescript-eslint/parser": "^5.8.0",
"coveralls": "^3.0.0",
"eslint": "^7.32.0",
"eslint-config-standard": "^16.0.3",
"eslint-plugin-import": "^2.25.3",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-promise": "^5.1.1",
"nyc": "^15.1.0",
"tape": "^4.11.0",
"tape-spawn": "^1.4.2",
"ts-node": "^10.4.0",
"typescript": "^4.5.4"
"@types/node": ">=16.11.7",
"@types/semver": ">=7.1.0",
"@types/tape": ">=4.2.27",
"@types/tmp": ">=0.2.0",
"@typescript-eslint/eslint-plugin": ">=5.0.0 <6.0.0",
"@typescript-eslint/parser": ">=5.0.0 <6.0.0",
"coveralls": ">=3.0.0",
"eslint": ">=7.12.1 <8.0.0",
"eslint-config-standard": ">=16.0.3 <17.0.0",
"eslint-plugin-import": ">=2.25.0",
"eslint-plugin-node": ">=11.1.0",
"eslint-plugin-promise": ">=5.0.0 <7.0.0",
"nyc": ">=15.0.0",
"tape": ">=4.11.0",
"tape-spawn": ">=1.0.0",
"ts-node": ">=10.0.0",
"typescript": ">=4.2.2 <5.0.0"
Comment on lines +62 to +74
Copy link
Member Author

Choose a reason for hiding this comment

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

Typescript, eslint and their plugins here are restricted only because they have issues on node.js 12 and 14. There are failures due to the use of ?? operator or its variants. On the other hand there are no issues on the latest node.js.

Not sure how to handle this. I guess node.js 12 and 14 are past EOL now so it may be ok to just remove the restriction. The issue is that our tests fail, so we would have to patch package.json in our tests.

Another possibility might be to just drop official v12 and v14 support.

},
"nyc": {
"exclude": [
Expand Down
2 changes: 1 addition & 1 deletion smtsolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function solve (query, solver) {
encoding: 'utf8',
maxBuffer: 1024 * 1024 * 1024,
stdio: 'pipe',
timeout: timeout // Enforce timeout on the process, since solvers can sometimes go around it.
timeout // Enforce timeout on the process, since solvers can sometimes go around it.
}
).toString();
} catch (e) {
Expand Down
3 changes: 1 addition & 2 deletions solc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const commanderParseInt = function (value) {
program.name('solcjs');
program.version(solc.version());
program
.option('--version', 'Show version and exit.')
.option('--optimize', 'Enable bytecode optimizer.', false)
.option(
'--optimize-runs <optimize-runs>',
Expand Down Expand Up @@ -201,7 +200,7 @@ const cliInput = {
}
}
},
sources: sources
sources
};
if (program.verbose) { console.log('>>> Compiling:\n' + toFormattedJson(cliInput) + '\n'); }
const output = JSON.parse(solc.compile(JSON.stringify(cliInput), callbacks));
Expand Down
8 changes: 4 additions & 4 deletions test/smtcallback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ tape('SMTCheckerCallback', function (t) {
const inputJSON = JSON.stringify({
language: 'Solidity',
sources: input,
settings: settings
settings
});

let tests;
Expand Down Expand Up @@ -192,7 +192,7 @@ tape('SMTCheckerCallback', function (t) {
expectations: expected,
solidity: { test: { content: preamble + source } },
ignoreCex: source.includes('// SMTIgnoreCex: yes'),
engine: engine
engine
};
}

Expand All @@ -214,7 +214,7 @@ tape('SMTCheckerCallback', function (t) {
const engine = test.engine !== undefined ? test.engine : 'all';
settings = {
modelChecker: {
engine: engine,
engine,
solvers: [
'smtlib2'
]
Expand All @@ -225,7 +225,7 @@ tape('SMTCheckerCallback', function (t) {
JSON.stringify({
language: 'Solidity',
sources: test.solidity,
settings: settings
settings
}),
// This test needs z3 specifically.
{ smtSolver: smtchecker.smtCallback(smtsolver.smtSolver, z3HornSolvers[0]) }
Expand Down
2 changes: 1 addition & 1 deletion test/smtchecker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ tape('SMTCheckerWithSolver', function (t) {
const input = {
language: 'Solidity',
sources: source,
settings: settings
settings
};

const output = JSON.parse(solc.compile(JSON.stringify(input)));
Expand Down
2 changes: 1 addition & 1 deletion translate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function translateErrors (ret, errors) {
type = 'Error';
}
ret.push({
type: type,
type,
component: 'general',
severity: (type === 'Warning') ? 'warning' : 'error',
message: errors[error],
Expand Down
4 changes: 2 additions & 2 deletions wrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ function compileStandardWrapper (compile, inputRaw, readCallback) {

// Try to wrap around old versions
if (!isNil(compile.compileJsonCallback)) {
const inputJson = JSON.stringify({ sources: sources });
const inputJson = JSON.stringify({ sources });
const output = compile.compileJsonCallback(inputJson, optimize, readCallback);
return translateOutput(output, libraries);
}

if (!isNil(compile.compileJsonMulti)) {
const output = compile.compileJsonMulti(JSON.stringify({ sources: sources }), optimize);
const output = compile.compileJsonMulti(JSON.stringify({ sources }), optimize);
return translateOutput(output, libraries);
}

Expand Down