Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

bump dependencies #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christopheranderson
Copy link

This PR's intent is to resolve audit warnings from npm.

I do not believe there is a need to release a security hotfix because the dependency issues involve dev dependencies, but I am not confident I know how binaries are distributed. If css-loader is distributed with duffle-bag binaries, impact should be evaluated more closely.

Changes

  • bumps css-loader to 3.2.0 to resolve audit issue (involved breaking change to css-loader apis)
  • adjusts webpack config (dev and prod) to resolve breaking changes in css-loader
  • bumps semantic-ui-sass to 2.4.1 to resolve build issue after moving to css-loader
  • adjusts app.global.scss to adjust the path for images directory in semantic-ui-sass

@itowlson
Copy link
Contributor

@christopheranderson Thanks for this! The context is that no, dev dependencies are not distributed with binaries -- so I agree no fix is needed on security grounds - but this repo is used as a code generation template for customer programs, and it is a poor experience for them to do a codegen, npm install their new repo, and find themselves confronting hard-to-evaluate security warnings.

Copy link
Contributor

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

@christopheranderson This looks like it works fine in npm run dev but something seems to go wrong when built into a binary. Here's how the first screen looks under npm run dev (in master or your PR), or in the binary generated by npm run package in master:

image

But if I check out this PR and do npm run package, then run the resulting binary from the release folder, then the first screen looks like this:

image

I'm on Windows; let me know if you don't see the problem on your OS.

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

Successfully merging this pull request may close these issues.

2 participants