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

fix(developer): work around devDependencies bug in npm #6074

Merged
merged 6 commits into from
Dec 22, 2021

Conversation

mcdurdin
Copy link
Member

Relates to npm/cli#2921

This issue arises after moving from npm 6.x to 7.x or later. As the build agents are still on 6.x, we have not yet seen this issue there.

As part of the release build for Keyman Developer, we bundle up some files and do some work in a temp folder to prepare folders for release. Even though we are only working with dependencies (e.g. using npm install --production), packages under devDependencies are still wrongly being checked by NPM. As we have two modules present in devDependencies that are only available when in the repo path, we need to remove them before attempting to continue. Yuck!

ref: npm/cli#3975 (comment)
ref: npm/cli#2921

@keymanapp-test-bot skip

Relates to npm/cli#2921

This issue arises after moving from npm 6.x to 7.x or later. As the
build agents are still on 6.x, we have not yet seen this issue there.

As part of the release build for Keyman Developer, we bundle up some
files and do some work in a temp folder to prepare folders for release.
Even though we are only working with `dependencies` (e.g. using `npm
install --production`), packages under `devDependencies` are still
wrongly being checked by NPM. As we have two modules present in
`devDependencies` that are only available when in the repo path, we need
to remove them before attempting to continue. Yuck!

ref: npm/cli#3975 (comment)
ref: npm/cli#2921
@mcdurdin mcdurdin added this to the A15S20 milestone Dec 21, 2021
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Dec 21, 2021

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

This made the string processing needed much simpler than
when it was embedded within the Makefile, where horrible
escaping proved necessary.
developer/js/bundle.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@rc-swag rc-swag left a comment

Choose a reason for hiding this comment

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

lgtm

@mcdurdin mcdurdin merged commit 4293f97 into master Dec 22, 2021
@mcdurdin mcdurdin deleted the fix/developer/npm=devdependency-bug branch December 22, 2021 23:02
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 15.0.172-alpha

mcdurdin added a commit that referenced this pull request Jan 10, 2022
Fixes a build failure after #6074 was merged. The final grep statement
in the line

  cat package.json | grep -v "@keymanapp/models-templates" | grep -v "@keymanapp/models-wordbreakers" > package.json

would fail to find anything to exclude, and return an error value, so
the script would fail, as we use `set -e`. We didn't care that it didn't
find anything to exclude... so we will ignore the return value.
mcdurdin added a commit that referenced this pull request Jan 16, 2022
mcdurdin added a commit that referenced this pull request Jan 17, 2022
mcdurdin added a commit that referenced this pull request Jan 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants