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

Update html/js libraries via npm #1993

Merged
merged 5 commits into from Jun 16, 2022
Merged

Conversation

cmonfortep
Copy link
Contributor

@cmonfortep cmonfortep commented Jun 9, 2022

Task/Issue URL: https://app.asana.com/0/414730916066338/1202412970169527/f

Description

Proposal to stop using submodules for html/js libraries.
As a first test, we will migrate autofill to the new approach.

Steps to test this PR

Ensure autofill works

  • checkout this branch
  • ensure features using autofill js continue to work (e.g: email tooltip)

Manual npm update

  • checkout this branch
  • Go to package.json and change the version number to a previous release (e.g: 4.5.0)
  • run npm update
  • ensure files inside node_modules have been updated

Validate your IDE don't show the submodule anymore

  • checkout this branch
  • open your IDE ensure autofill submodule has been removed
  • Ensure IDE doesn't complain about it

UI changes

Before After
!(Upload before screenshot) (Upload after screenshot)

app/build.gradle Outdated
@@ -36,7 +36,7 @@ android {
main {
java {
resources {
srcDirs += files("$projectDir/../submodules/autofill/dist/".toString())
srcDirs += "$projectDir/../node_modules/@duckduckgo/autofill/dist/".toString()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will ensure only files inside /dist will be included when we build the app.

"test": "echo \"Error: no test specified\" && exit 1"
},
"dependencies": {
"@duckduckgo/autofill": "github:duckduckgo/duckduckgo-autofill#4.6.0"
Copy link
Contributor Author

@cmonfortep cmonfortep Jun 9, 2022

Choose a reason for hiding this comment

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

Here's where we will change the library versions.

uses: actions/setup-node@v1
with:
node-version: 16.x
- name: Npm update
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This Github Action will run npm and autocommit the changes.

@cmonfortep cmonfortep marked this pull request as ready for review June 15, 2022 15:28
@CDRussell
Copy link
Member

@cmonfortep I just switched to autofill v4.6.0 and created a merge conflict for you. You'll need to resolve it on your branch even though that's going to be removed

Copy link
Member

@CDRussell CDRussell left a comment

Choose a reason for hiding this comment

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

All tested and working well. Great job, this is a fantastic change. 🎉

@cmonfortep cmonfortep merged commit 7c24d57 into develop Jun 16, 2022
@cmonfortep cmonfortep deleted the hackdays/cristian/npm_updater branch June 16, 2022 12:38
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

2 participants