Skip to content

Conversation

@alexsch01
Copy link
Contributor

@alexsch01 alexsch01 commented Nov 24, 2025

Being able to override CONNECTION_DRIVER (now DEFAULT_CONNECTION_DRIVER) variable for mssql/msnodesqlv8

  • while maintaining compatibility with configs with driver value "msnodesqlv8"
  • if config.driver is a truthy value and not equal to "msnodesqlv8", it will use that specific driver

Changed README, don't know if GitHub Pages site needs an update


const sql = require('mssql/msnodesqlv8');

const config = {
  server: "MyServer",
  database: "MyDatabase",
  options: {
    trustedConnection: true, // Set to true if using Windows Authentication
    trustServerCertificate: true, // Set to true if using self-signed certificates
  },
  driver: "ODBC Driver 18 for SQL Server",
};

(async () => {
  try {
    await sql.connect(config);
    const result = await sql.query`select TOP 10 * from MyTable`;
    console.dir(result);
  } catch (err) {
    console.error(err);
  }
})();

@alexsch01
Copy link
Contributor Author

alexsch01 commented Nov 24, 2025

@dhensby let me know if you are ok with this feature

In CI - before running mssql with msnodesqlv8, I run a script that patches this package to use the "ODBC Driver 18 for SQL Server" driver

@dhensby
Copy link
Collaborator

dhensby commented Nov 28, 2025

Thanks @alexsch01 - my main concern now is that this doesn't have any test coverage, so there's no regression detection for the futue. Would you be able to add a test to show that we can update the config with a different driver and it still works?

@alexsch01
Copy link
Contributor Author

@dhensby I added a test, can you let the checks run?

@dhensby
Copy link
Collaborator

dhensby commented Dec 1, 2025

The last 2 commits will fail commit linting - they can be squashed into the first commit

@alexsch01
Copy link
Contributor Author

@dhensby hey can you let checks run?

simplify

tests for config.driver

revert partially of the test change

Update msnodesqlv8.js

revert workflow change

change test for Windows

Update msnodesqlv8.js

fix: msnodesqlv8 test
@alexsch01
Copy link
Contributor Author

alexsch01 commented Dec 2, 2025

one more time, sorry @dhensby

@alexsch01
Copy link
Contributor Author

All tests passed! @dhensby

@dhensby dhensby merged commit 61fce50 into tediousjs:master Dec 2, 2025
43 checks passed
@dhensby
Copy link
Collaborator

dhensby commented Dec 2, 2025

Thanks so much for the work on this and putting up with the tedium of the commit conventions!

@alexsch01
Copy link
Contributor Author

alexsch01 commented Dec 2, 2025

Thanks so much for the work on this and putting up with the tedium of the commit conventions!

no problem, glad the issue this PR fixes is resolved

@github-actions
Copy link

github-actions bot commented Dec 2, 2025

🎉 This PR is included in version 12.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants