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.

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

updated all packages to latest #15

Merged
merged 1 commit into from Nov 24, 2022
Merged

Conversation

vision10
Copy link
Collaborator

No description provided.

@Catbuttes Catbuttes merged commit b322101 into Catbuttes:master Nov 24, 2022
@mlent
Copy link

mlent commented Nov 25, 2022

Hey friends - wanted to let you know, making this major upgrade in a minor release broke our build.

It would be better if something like this were in a major version release, so that it didn't get silently pulled in via subdependencies. As in the latest version of Axios they also changed the import structure.

For anyone else who happens to discover this today, what I've tried so far:

  • In package.json, using Yarn's selective dependencies to pin soap/axios-ntlm to 1.3.0 which didn't work due to our setup

EDIT:

I ended up forking the upstream package, hard resetting it to an old commit, updating the package.json to rely on axios-ntlm-1.3.0, and building and committing the compiled files. Then installing via the git repo instead of npm.

In case anyone else needs a temporary solution as fast as possible.

@vision10
Copy link
Collaborator Author

axios-ntlm uses
import axios, { AxiosError, AxiosInstance, AxiosRequestConfig, AxiosResponse } from 'axios';

axios docs latest
import axios, {isCancel, AxiosError} from 'axios';

It seems the same, what changed in axios import structure ?

@mlent
Copy link

mlent commented Nov 25, 2022

@vision10 I'm guessing it's this: https://github.com/axios/axios/releases/tag/v1.2.0

changed: refactored module exports axios/axios#5162
change: re-added support for loading Axios with require('axios').default axios/axios#5225

@louislam
Copy link

I am still using "axios": "~0.27.0", after update axios-ntlm to 1.3.1, it throws such error.

Here is the full error log for your reference:
https://github.com/louislam/uptime-kuma/actions/runs/3542675989/jobs/5954006102

  /home/runner/work/uptime-kuma/uptime-kuma/node_modules/axios-ntlm/node_modules/axios/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import axios from './lib/axios.js';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      15 | const postgresConParse = require("pg-connection-string").parse;
      16 | const mysql = require("mysql2");
    > 17 | const { NtlmClient } = require("axios-ntlm");

Since the error throws from /node_modules/axios-ntlm/node_modules/axios which is a sub-depdenency. Which also means there are two axios in my project.

Since axios-ntlm is somehow like a plugin of axios, having a sub-depdenency may not be a good idea.

I would suggest that using peerDependencies to prevent the sub-depdenency issue.

For more details, please read:
https://nodejs.org/es/blog/npm/peer-dependencies/

@vision10
Copy link
Collaborator Author

would updating the package to version 1.4.0 solv the problem?

@mlent
Copy link

mlent commented Nov 29, 2022

@vision10 version 1.4.0 of what? you mean making another release of this package which reverts the change?

@Loksly
Copy link

Loksly commented Jan 3, 2023

same problem, also related to axios and soap, like #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants