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

Fabrica Software Updates #675

Merged
merged 105 commits into from
Feb 8, 2023
Merged

Fabrica Software Updates #675

merged 105 commits into from
Feb 8, 2023

Conversation

svogt0511
Copy link
Contributor

@svogt0511 svogt0511 commented Nov 14, 2022

Purpose

Upgrade Fabrica addons and core. Take care of security issues. There will be a follow-on task for the next tech debt addressing anything we could not do in this week.

Current status: Fabrica has been upgraded to Ember v3.16. 3rd-party add-ons have been upgraded as much as possible.
Current security issues have been addressed. All tests are passing. Cypress testing has been upgraded from v7 to v10.

Latest Vercel Deployment: https://bracco-bgpoy2xxb-datacite.vercel.app/

closes: #629

Approach

Followed the recommended update process.

  • Update 3rd-party add-ons.
  • Move the ember core/ember-data/ember-cli to v3.15 so that we can use ember tools for upgrading to the next LTS (v3.16).
  • Move to the next Ember.js LTS - v3.16. This is done using the following commands:
  ember-cli-update --to 3.16
  ember-cli-update --run-codemods
  • Use yarn resolutions to deal with remaining security critical/high level issues reported by GitHub dependabot or yarn audit.
  • Fix any problems with yarn, the ember build, or those found with the DataCite test suites.
  • Fix or document Fabrica bugs discovered during this process.

Follow-On Tasks

These are for following technical debt weeks:

  • Fix deprecations listed in config/deprecation-workflow.js
  • Try recreating yarn.lock to address any deprecation warnings (rm yarn.lock, then yarn install).
  • Fix codemods that could not be done automatically in this upgrade (see codemods.log).
  • Fix "loose" warnings in the Fabrica build (see the build log).
  • Run 'yarn audit' and check for dependabot security issues that should be fixed.
  • Upgrade Cypress to V11. Implement component tests. Implement data factories and mocking for api requests.

Future Upgrades - Ember

We can try going directly to Ember v3.28. However it might be an easier path to follow the above process and move through the next LTS upgrades until we get to 3.28, bearing in mind that we might have problems with one crucial module (ember-data-model-fragments) which has been reported to have problems around Ember v3.28.

At v3.15 there is the beginning of the move from Ember "Classic" to Ember "Octane". There are numerous deprecations, and beyond v3.28, at v4, those deprecated features are dropped. There are a number of major changes to the framework, including Typescript with native Ember classes, improved components, tracked properties and a number of other new features. An upgrade guide and a blog post with pointers to more information, and the Ember Octane vs Classic Cheat Sheet.

Future Upgrades - Node

It looks like we can update Node pretty easily by upgrading our GitHub workflows and Vercel build to use Node v16, and additionally adding the following to package.json:

# in package.json
...
 "engines": {
    "node": "16.*"
  },
...

I already tried it and it seemed to work. However there is a compatibility matrix for node/ember which states that we are already ahead in terms of Node version so I decided to stick with 14 until we can do further testing in another technical debt week.

Future Upgrades - Yarn

Perhaps move to the new Yarn from Yarn Classic. Yarn classic works, but is pretty old and barely maintained. We don't need to change it at present but might want to upgrade later to take advantage of new features and better/faster build process in the new Yarn.

Future Upgrades - Cypress Testing

There is a lot to be done here. End-to-end testing needs to be extended. The move to V10 of Cypress makes component testing possible and seems to have improved the way the test suite runs. Additionally testing should be extended with mock api calls and data factories.

Learning

PRE-DEPLOYMENT TO DO

  • Set release tag in package.json to the latest release tag and set that as the release tag when creating a release.

POST-DEPLOYMENT TO DO

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • Software updates (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

@cypress
Copy link

cypress bot commented Nov 14, 2022

Passing run #639 ↗︎

0 77 0 0 Flakiness 0

Details:

Merge f3d81a4 into 0884b94...
Project: bracco Commit: 5f12d1dae6 ℹ️
Status: Passed Duration: 03:10 💡
Started: Feb 8, 2023 4:43 AM Ended: Feb 8, 2023 4:46 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@svogt0511 svogt0511 force-pushed the fabrica-sw-update-main branch 4 times, most recently from 2b4dce9 to 548f7bd Compare November 28, 2022 01:59
@cypress
Copy link

cypress bot commented Dec 2, 2022

Passing run #620 ↗︎

0 77 0 0 Flakiness 0

Details:

Fabrica-sw-update: option to run vercel build without cache.
Project: bracco Commit: 708887e5b8
Status: Passed Duration: 03:17 💡
Started: Feb 1, 2023 6:14 PM Ended: Feb 1, 2023 6:17 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@svogt0511 svogt0511 force-pushed the fabrica-sw-update-main branch from 98cebfe to ac755e3 Compare December 8, 2022 01:46
Copy link
Contributor

@jrhoads jrhoads left a comment

Choose a reason for hiding this comment

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

This is a lot of work.
Fantastic job!!!

@svogt0511 svogt0511 force-pushed the fabrica-sw-update-main branch 8 times, most recently from 5e690d1 to 708887e Compare February 2, 2023 20:27
Copy link
Contributor

@richardhallett richardhallett left a comment

Choose a reason for hiding this comment

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

Ye looks good, however for deploy have a rollback plan in case of a weird compatability issue.
However if its a minor thing id rather we fix adhoc.

@svogt0511 svogt0511 self-assigned this Feb 7, 2023
@svogt0511
Copy link
Contributor Author

svogt0511 commented Feb 7, 2023

Loose Mode Warnings

A number of 'loose mode' warnings happen during the fabrica build during the transpilation process by babel.

Loose mode explanation: http://hzoo.github.io/babel.github.io/docs/advanced/loose/

The build process:

# clone the repo, then:

yarn install

# then build with one of the following:

ember serve
./node_modules/.bin/ember build --output-path="test_build" --environment=development
./node_modules/.bin/ember build --output-path="test_build" --environment=production

The first build using the above sequence results in a number of warnings as below. Subsequent builds do not show these messages, although I think you can see the messages again with yarn cache clean and then build.

Though the "loose" option was set to "false" in your @babel/preset-env config, it will not be used for @babel/plugin-proposal-private-property-in-object since the "loose" mode option was set to "true" for @babel/plugin-proposal-class-properties.
The "loose" option must be the same for @babel/plugin-proposal-class-properties, @babel/plugin-proposal-private-methods and @babel/plugin-proposal-private-property-in-object (when they are enabled): you can silence this warning by explicitly adding
    ["@babel/plugin-proposal-private…

These warnings can also be seen in a test build log on github: https://github.com/datacite/bracco/actions/runs/4078392258

The recommended the configuration to silence the warnings is:

# in babel.config.json:
{
  "presets": [
    "@babel/preset-env"
  ],
  "plugins": [
    ["@babel/plugin-proposal-private-property-in-object", { "loose": true }],
    ["@babel/plugin-proposal-class-properties", { "loose": true }],
    ["@babel/plugin-proposal-private-methods", { "loose": true }]
  ]
}

This work-around does not work. It causes an error, very much like the warnings, but which stops the build.

There is currently an open issue related to this against ember-cli-babel - emberjs/ember-cli-babel#415 I have tried backing out the version of ember-cli-babel and @babel-preseet-env to arounc v7.0.0 with no luck. Do not want to go back to v6.

I have also tried just generally turning on loose mode, but it seems to have no affect.

I think this is just a conflict between various module requirements and, as it says, they are just ignoring our settings, which should be ok, and they are warning us to that effect.

This has upgrade has been tested and will be deployed to staging for a test period of a few days for anybody to try. It can be easily backed out if needed, however, I wanted to make sure this is documented here.

@svogt0511
Copy link
Contributor Author

Deploying to staging env.

@svogt0511 svogt0511 merged commit 3fb1308 into main Feb 8, 2023
@svogt0511 svogt0511 deleted the fabrica-sw-update-main branch February 8, 2023 04:51
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.

Fabrica Software Upgrade
4 participants