Skip to content

Comments

[4.0] npm update 19 sep#30693

Closed
brianteeman wants to merge 1 commit intojoomla:4.0-devfrom
brianteeman:rhh1
Closed

[4.0] npm update 19 sep#30693
brianteeman wants to merge 1 commit intojoomla:4.0-devfrom
brianteeman:rhh1

Conversation

@brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Sep 19, 2020

main change is an update to caniuselite
the rest are bug or security fixes in line with semver

main change is an update to caniuselite
the rest are bug or security fixes in line with semver
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Sep 19, 2020
@richard67
Copy link
Member

richard67 commented Sep 20, 2020

@brianteeman When using the changed file from this PR on Linux and running npm ci, I get errors Error: Cannot find module 'is-obj'.

When running npm install instead of npm ci, the package-lock.json is changed, and the missing reference to is-obj is added, see following diff with the file from this PR on the left hand side and the one updated by npm on the right hand side:

2020-09-20_01

But before that difference, the comparison shows another one, and that could be the reason why you did not have the problem with unresolved is-obj, and I had:

2020-09-20_02

Beside these two differences no more differences are shown.

You can download my package-lock from the comparison above here: https://test5.richard-fath.de/package-lock.json.

@wilsonge Any idea?

@richard67
Copy link
Member

Checking where the first difference comes from, i.e. the one mentioned as second in my previous comment, I see that it comes from this PR:

2020-09-20_03

I suggest you use my file, see link in my previous comment.

@brianteeman
Copy link
Contributor Author

from my understanding of the difference between npm ci and npm install this PR is correct and your finding is expected

@richard67
Copy link
Member

@wilsonge Please check and confirm.

@richard67
Copy link
Member

@brianteeman As I wrote, when using the changed file from this PR on Linux and running npm ci, I get errors Error: Cannot find module 'is-obj'. I don't think that's expected. And your PR shows exactly that dependency with empty resvolved where without your PR there are correct entries.

@brianteeman
Copy link
Contributor Author

Please read about the difference between npm install and npm ci

@richard67
Copy link
Member

Please read about the difference between npm install and npm ci

I know that difference very well, but maybe you should read a bit npm docs to understand that your PR introduces an unresolved reference, as clearly shown in my screenshot above.

Normally I should just give a negative test result, but I know you would not accept that.

So I shut up at this point and stop to bother you.

Shall someone else handle that.

Meanwhile I think about stop contibuting and leave all teams because I can do better stuff in my spare time than being treated like a little school boy here.

@rdeutz
Copy link
Contributor

rdeutz commented Sep 21, 2020

@brianteeman I would like to see in the furture that you treat people like you to be treated. There is no need that we make it more complicted and less nice as needed. We all do our best here to make progress.

@brianteeman
Copy link
Contributor Author

I still dont understand what is wrong with this pr. from everything I read about the difference between npm ci and npm install the changes are expected

@brianteeman brianteeman deleted the rhh1 branch September 21, 2020 14:14
@richard67
Copy link
Member

As I said, there are lots of Error: Cannot find module 'is-obj' issued by npm about the unresolved reference to is-obj, and that is caused by the following change in your PR:

2020-09-20_03

And with my version of the file respective the new PR #30713 that doesn't happen.

I don't have an explanation why that happened with your PR. Maybe something OS specific (I worked on Linux).

It looks a bit as if npm had been run before with a "--no-dev" option or similar, but I don't think that's the case because why should you have done that.

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

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants