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

[dependencies] Remove dependency to trim newlines #18468

Closed

Conversation

Aaronkala
Copy link

@Aaronkala Aaronkala commented Jun 13, 2022

Issue: #18287 (also related to #17220)

What I did

Some notes: "default-browser" is an ESM package so it was causing some issues in jest. I added the new ESM packages to the transformIgnorePatterns but I'm not sure if this is the best approach. Suggestions welcome here :)

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

the "x-default-browser" npm package has a dependency to "[email protected]", which contains a high severity security vulnerability (see: https://snyk.io/test/npm/trim-newlines/1.0.0).

This commit replaces "x-default-browser" with "default-browser".
@nx-cloud
Copy link

nx-cloud bot commented Jun 13, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 2ea387c. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@socket-security
Copy link

Socket Security – Alert

🧐 Potential typosquat detected

A package that was added in the pull request has a name similar to other popular packages.

Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages.

Installed package 📎 Did you mean...? Found in
default-browser **x-**default-browser (3.5 thousand times more downloads) lib/core-server/package.json

Powered by socket.dev

@@ -50,7 +50,9 @@ module.exports = {
'^.+\\.[jt]sx?$': '<rootDir>/scripts/utils/jest-transform-js.js',
'^.+\\.mdx$': '@storybook/addon-docs/jest-transform-mdx',
},
transformIgnorePatterns: ['/node_modules/(?!lit-html).+\\.js'],
transformIgnorePatterns: [
'/node_modules/(?!lit-html|default-browser|bundle-name|run-applescript).+\\.js',
Copy link
Author

Choose a reason for hiding this comment

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

A better solution to this is welcome.

default-browser, bundle-name and run-applescript are ESM modules

Copy link

Choose a reason for hiding this comment

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

I used dynamic import to solve this ESM thing.

@Aaronkala Aaronkala changed the title Remove dependency to trim newlines [dependencies] Remove dependency to trim newlines Jun 13, 2022
@MichaelArestad MichaelArestad added maintenance User-facing maintenance tasks security labels Jun 13, 2022
@kevcube
Copy link

kevcube commented Jun 29, 2022

@Aaronkala whoops, I just duplicated this in #18594 😢

@ndelangen
Copy link
Member

@Aaronkala Thank you for this PR..

The issue relating the security of x-default-browser was recently addressed in another PR: #18277

import getDefaultBrowser from '@aw-web-design/x-default-browser';

So I'll close this PR.

Thank you for your effort, time and energy you put into creating this PR. We truly appreciate it!

@ndelangen ndelangen closed this Jul 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants