Upgrade Zola Ember & Set Node Version

[allthesignals]

This issue is closed when Ember is version 3.19 and the app builds in Netlify for Node 14/16

Development issues

"yarn add node-sass never finish"

See workaround: https://github.com/yarnpkg/yarn/issues/5129#issuecomment-463726676

npm i node-sass
yarn add node-sass
yarn install

TODO: should switch out labs-ui, rework the sass-based styling dependency, and consider postcss to compile sass. Or, consider a static build of labs-ui styles... then do some overrides where necessary...

Cannot find module '@babel/plugin-proposal-private-property-in-object'

Simply yarn add this dependency

Font Awesome issues

Build Error (fontawesome-svg-core): acorn.Parser.extend is not a function

Currently unknown....

General Direction so far

A huge issue and barrier here is node-sass and foundation. Foundation-Sites has some known SemVer "violations" which make upgrading difficult and maintenance unsustainable.

Further, node-sass is a C dependency, which is high overhead for setting u pthe project.

I dealt with this in ZAP Search frontend by pulling in the compiled "Labs UI" stylesheets as a single asset and building on top of that. Pre-processing was moved away from node-sass (deprecated) and towards PostCSS.

Because node-sass is deprecated (I believe it's now "Dart Sass" or something), we should move away from this and towards a high-level compiler (node-based). This will elminate the need for a C compiler for SASS (which is wild). I think for some orgs with extremely complicated stylesheets, a fast compiler is very nice, but that is not our situation currently.

What this means is needing to move some things away from the labs-ui shared addon. I dealt with this in ZAP Search Front End by pulling in all the shared components in the consumer repo. This creates duplication, yes, but reduces overheard. I've heard this advice from other organizations dealing with addon-related upgrade woes.

I'm open to suggestions on this. The other option is to "fix" the labs-ui dependency, but I'm concerned with the unnecessary overhead this may create. I'll spike this path to see if it's a possible "fix-all" for consuming repos!

[TylerMatteo]

I totally hear you on upgrading labs-ui being a headache but I really do not want us pulling stuff out of shared repositories. The Labs portfolio doesn't do the best job of "sharing code" (be it through npm packages, RESTful apis, or other means) as it stands today, so I'd rather not add to that by pulling out and duplicating the shared code we do have. You're on the right track with the sass stuff but maybe I can clear things up - in this context, "SASS" refers to a language-agnostic "spec" that has been implemented in several languages. As you pointed out, node-sass is a thin wrapper around the C implementation. "Dart Sass" is an implementation written in Dart that has been the "default" one for a little while now - so it gets new features the soonest and generally has the best support. It's probably worth looking into if we can move to this package which is the JS distribution of Dart Sass (as opposed to the one we currently use which comes from another npm package named node-sass)

[TylerMatteo]

Fwiw, it looks like our node-sass dependency comes from broccoli-sass-source-maps which, in turn, comes from ember-cli-sass version 7.2.0. That's a whopping 4 major version behind the current version of ember-cli-sass but the most recent version of that dep depends on sass, the dart-sass-based package.

[allthesignals]

You're on the right track with the sass stuff but maybe I can clear things up - in this context, "SASS" refers to a language-agnostic "spec" that has been implemented in several languages. As you pointed out, node-sass is a thin wrapper around the C implementation.

Yes this is my understanding.

The Labs portfolio doesn't do the best job of "sharing code" (be it through npm packages, RESTful apis, or other means) as it stands today, so I'd rather not add to that by pulling out and duplicating the shared code we do have.

That's what I'm trying first.

It's probably worth looking into if we can move to this package which is the JS distribution of Dart Sass (as opposed to the one we currently use which comes from another npm package named node-sass)

I'll try that first.

That's a whopping 4 major version behind the current version of ember-cli-sass but the most recent version of that dep depends on sass, the dart-sass-based package.

That's promising.

[allthesignals]

@TylerMatteo so ideally we will end up with this:

Labs-UI shared code will introduce correct Sass deps (Dart-based ember-cli-sass) and respective updates will happen throughout the portfolio.

The reason I'm exploring other paths is that the main goal as far as I understand is to upgrade things to Ember 3.19. However, I'm seeing a number of hurdles introduced by these sass dependencies and looking to simplify in hopes of successfully completing the upgrade.

[TylerMatteo]

Yeah you're correct about the upgrade being the main objective, I just want us to at least make a serious effort to do so without introducing other forms of tech debt into the portfolio. That being said, I'm not necessarily expecting you to reintegrate dependencies where they've already been split out, such as ZAP search - my hope is that would be relatively easy for the team to handle once all the upgrades are complete.

[allthesignals]

Totally separate, but wow, layers-api:

I don't even know if that application is set up to support compression...

[allthesignals]

Okay, pulling down labs-ui to develop locally and hopefully solving some of this stuff

[allthesignals]

Promising: swapping in the new dart-baesd ember-cli-sass drops node-gyp and node-sass as expected:

I've opened a PR for the upgrade of labs-ui, but I'm going to test it locally as well to see how a version bump of it might affect consumer apps: https://github.com/NYCPlanning/labs-ui/pull/78.

~Having a little trouble testing this locally with yarn link... I recall symlinking doesn't actually fully simulate the way these packages work...~ ~edit: oh~ still not able to test consuming app locally... I vaguely recall a tool that did this... here it is... this works a little better when debugging dependency/version issues in packages. Okay, that's revealing that I need to run dependency-lint on labs-zola to figure out why it's resolving for two different versions of broccoli-sass-source-maps. More to come...

[allthesignals]

Stepping back and running dependency lint here's where we are:

I have a branch for another missing dependency that might've popped up from some dependency drift or node versioning (who can say): mg/escape-dependency-hell

Fixed dependency lint issue by upgrading fort-awesome...

Am I being trolled?

It's crazy, it almost seems to resolve stuff non-deterministically (regenerating yarn.lock for example). No matter what it seems to resolve @fortawesome/ember-fontawesome with different versions. Fun! Going to punt on that path for now...

Okay one source of conflicting versions: labs-ui and labs-ember-search both specific ember-power-select as dependencies. Depending on the version, labs-ui asks for power-select 4. ember-search is still on version 2. Hence, upgrades lead to build time errors... turning towards upgrading ember-search now...

[allthesignals]

All sorts of strange behavior on labs-ember-search, and that one has a small number of dependencies. I'm going to keep chipping away that one and see what I learn...

The (ember-cli-search) package's current ember-cli version requires node 10. Upgrading ember-cli griefs me so far...

Made some progress:

1. On Node 12
2. On ember-cli 3.10 (no more Active LTS warning)
3. Moved deps that didnt' need to be in "dependencies" into "devDevependencies"
4. Upgrade to ember 3.11

In order to upgrade ember-power-select, I will need to upgrade ember-source due to the ember-basic-dropdown dependency...

See here

Update: got ember upgraded and also ember-power-select!

Draft PR for labs-ember-search

[TylerMatteo]

Thanks for documenting all these trials and tribulations 😆 The dependency path to ember-basic-dropdown definitely rings some bells to the last time I tried to untangle this. Sounds like you're making some progress?

[TylerMatteo]

As you're digging into labs-ember-search, just want to point you at this PR I did back in December. I had to revert some upgrades so that I could release a version that was still compatible with older apps but included some data-related updates we needed. Not sure if it's relevant to your work here, but I'd imagine you might end up re-implementing some of the changes I reverted in this PR so just wanted to make sure you're aware.

It looks like the changes I reverted got it all the way up to ember-cli 3.18, so probably worth looking at. We'll just have to make sure we don't inadvertently revert any of my changes related to what Carto datasets its pointing at but that should be easy enough to catch in code review.

[allthesignals]

@TylerMatteo okay, thank you for that context! I'm trying to avoid upgrading things too far. My goal is to test the two main dependencies in consuming apps before committing to them. I've upgraded to 3.11.

All that to say... I can see how all this dependency mess has kept everyone and everything "stuck" in a really bad way! Going to keep digging...

[allthesignals]

Let me know if you prefer I not document things here, just need to keep track of where I left things in case I need to switch context to something else.

Currently: using yalc to test labs-ember-search with other consuming apps and noting common pitfalls...

[allthesignals]

There's a 3rd dependency to worry about, too: ember-mapbox-composer.

Getting recurring issues with fortawesome, seeing this:

Build Error (broccoli-persistent-filter:Babel > [Babel: @fortawesome/ember-fontawesome]) in @fortawesome/ember-fontawesome/components/fa-icon.js

[BABEL] /Users/mattgardner/labs-zola/@fortawesome/ember-fontawesome/components/fa-icon.js: api.assumption is not a function (While processing: "/Users/mattgardner/labs-zola/node_modules/labs-ui/node_modules/@babel/plugin-transform-modules-amd/lib/index.js")

"Fix" is to rm -rf node_modules and reinstall everything.

Current "working" branch I'm using is mg/escape-dependency-hell-simplify...

Separately: https://github.com/NYCPlanning/labs-ember-search/pull/44 is a PR where I do gentle upgrades, only a few minor versions. Next step I need to clean it up by moving some things back into the "dependencies." For ember addons, the dependencies hash tells Ember which explicit dependencies it needs (as opposed to devDependencies).

[allthesignals]

Going from labs-ui 0.0.25 to 1+ is going to be pretty bad because labs-zola actually directly extends some of the components in there.

Two paths:

1. I'm going to see if I can fix the way it extends those components.
2. fork from labs-ui 0.0.25

[allthesignals]

Path 1 leads to some blockers that are usually solved by codemods, however, there is some behavior going on in the linked code below that I think will probably totally block a full upgrade to Ember 3.19:

https://github.com/NYCPlanning/labs-zola/blob/develop/app/templates/components/labs-ui/layer-groups-container.hbs#L1-L22

I'll see where I can get with this. I at least have a solution path towards 3.11.

Last ended on: It's necessary for soem reason to includer @glimmer/component as a dependency on consuming apps (something about font awesome). However, I can only get 0.0.14 (alpha.13) to work. No idea way. Going from labs-ui 0.0.25 to 1 breaks a bunch of functionality I think because it switches from classic component sytnax to native class syntax. Might be related to how glimmer deals with this at the addon level...

[TylerMatteo]

Let me know if you prefer I not document things here, just need to keep track of where I left things in case I need to switch context to something else.

I think documenting all this stream-of-consciousness stuff here is great. Once you have any cleaned up PRs that are ready to review, you can describe what you actually ended up doing in those (within reason, don't need a book for each PR).

I'm totally onboard with you doing multi-step upgrades, such as going to 3.11 before continuing on to 3.19 if you think that will be helpful. I'd imagine it might help break up the work. The extending thing in Zola is a little concerning. Do you think that is an artifact of a previous attempt to move some of that labs-ui code out of the library and duplicate it into Zola directly?

[allthesignals]

I think documenting all this stream-of-consciousness stuff here is great. Once you have any cleaned up PRs that are ready to review, you can describe what you actually ended up doing in those (within reason, don't need a book for each PR).

Great!

The extending thing in Zola is a little concerning. Do you think that is an artifact of a previous attempt to move some of that labs-ui code out of the library and duplicate it into Zola directly?

I think it's just an artifact of bad design... I think I wanted to use some composability with the existing labs-ui components (I wanted programmatic access to rendered children of "layer-group-containers", for example, "Zoning and Land Use":

but I think the problems are fixable. I'm going to try to keep things away from "refactor" zone and firmly inside of "codemod/syntax updates".

[allthesignals]

Note to self: assess ZoLa deprecation warnings if we get to 3.19.

[allthesignals]

Possible helpful tool: https://github.com/raineorshine/npm-check-updates#doctor-mode

[TylerMatteo]

When you feel like you have your thoughts straight (which I know is hard to do with this stuff), can you give me a brief summary of where you're at with all of this? In particular, I'm curious which labs-built dependency you think need upgraded before we can upgrade Zola (and, presumably, the other front ends). It sounds like at least labs-ember-search, labs-ui, and ember-mapbox-composer would be on that list? Any others?

[allthesignals]

I'm still in the midst of discovery mode so I won't have a concise answer for you on that just yet but give me some more time and I'll see where I've landed. There are a lot of moving parts (this is less straightforward because of the requirement to keep labs-built addons/dependencies rather than pulling them into consumer apps).

[TylerMatteo]

Just curious, have you tried pointing the apps at version 1.1.0 of labs-ui after upgrading them? It looks like Andy got it upgraded to ember cli 3.17 at one point (though that might not be helpful if you're trying to upgade to 3.11 first before continuing on to 3.19). Feel free to ignore me here if you were aware of this version but don't think it's helpful, just throwing it out there because I just came across it myself.

[allthesignals]

@TylerMatteo yeah, I tried that, the problem is that things were switched to pure glimmer components, which is a breaking change (hence the major semver). And the handlebars syntax for some of the extended components do not work with pure glimmer components. there's some deprecation stuff (like referencing actions with strings) that introducing glimmer components breaks... I don't know if that makes sense but basically the labs-ui 1+ version prematurely introduces a breaking change (glimmer) that ZoLa (after extending it) needs refactoring to support...

Separately, I thought of a different strategy. So, the thing I want to do is reduce the number of variables WHILE upgrading... because the labs addons are sometimes opionated about which dependencies are required, sometimes those are inappropriately specified in "dependencies," I am going to pull in (duplicate) the addons temporarily while upgrading ZoLa. That way I can focus on which depencies I know for sure that I'm dealing with, and not needing to deal with yalc in order to simulate the npm registry, lock-file re-generation, etc. Then, once I've upgraded Ember in ZoLa (fingers crossed), including any code changes, I'll be able to identify what needs to happen in the addons in order to have a stable upgrade. That way, I can extract that addon code BACK out into addons...

Anyway this is my current thinking.

[TylerMatteo]

I tried that, the problem is that things were switched to pure glimmer components, which is a breaking change (hence the major semver). And the handlebars syntax for some of the extended components do not work with pure glimmer components.

Gotcha. Is it correct to say that labs-ui could be upgraded to ember 3.19 without switching to pure glimmer components? In other words, did Andy switch to glimmer components so that he could upgrade or was it just taking advantage of a new feature that the upgrade enabled?

I am going to pull in (duplicate) the addons temporarily while upgrading ZoLa.

Seems reasonable enough to me. Maybe we can squash commits with the duplicated components before we go to master, just to keep the commit history less confusing.

[allthesignals]

Gotcha. Is it correct to say that labs-ui could be upgraded to ember 3.19 without switching to pure glimmer components? In other words, did Andy switch to glimmer components so that he could upgrade or was it just taking advantage of a new feature that the upgrade enabled?

The latter — taking advantage of new features. Within 3+, "classic" components are still backwards compatible so we can upgrade without switching to glimmer components.

Seems reasonable enough to me. Maybe we can squash commits with the duplicated components before we go to master, just to keep the commit history less confusing.

Sure, I'm SO FAR getting into a rhythm with the "ejected" labs-ui... upgrade 1 minor version, run tests, pass, repeat. (The only "test" that fails is DependencyLint). Knock on wood!

So far, I've only ejected labs-ui... this may help me uncover what the source of pain is so that I can go back and redo my commit history without the ejection. We shall see. This wil inform upgrades for the other apps.

[TylerMatteo]

The latter — taking advantage of new features. Within 3+, "classic" components are still backwards compatible so we can upgrade without switching to glimmer components.

Well, that's good to hear I suppose. Our team will have to keep in mind that, apparently, Ember will stop supporting v3 for Node versions after 16, which goes EoL in fall 2023.

[allthesignals]

What this upgrade process will do is it will start throwing a bunch of new deprecation warnings in the development console. The next step would be to address those to prepare the app to be upgraded to 4.

[allthesignals]

This is going well. Currently on 3.13.

[allthesignals]

https://github.com/NYCPlanning/labs-zola/pull/1059

Ember 3.12 upgrade available on this branch.

However, I'm in a bit of a catch-22: tests fail due to dependency-lint issues. This is somehow related to the way versions are specified on labs-ui and labs-ember-search: https://github.com/NYCPlanning/labs-ember-search/blob/master/package.json#L43 https://github.com/NYCPlanning/labs-ui/blob/27f75acec891f073067cbb907b7071596b7517d1/package.json#L22

I have tried to fix those issues, but they would seem to require upstream changes to labs-ui and labs-ember-search.

As a temporary stopgap, I could configure dependency lint to allow multiple versions.

Notes

1.

Unexpected token '.'

Stack Trace and Error Report: /var/folders/g0/l926clqx2zn2wgvvx_r8xs700000gp/T/error.dump.7e122204d4c2a78fe14fed00fc1e9d49.log
cleaning up...

Super unhelpful error! Stems from ember-cli-fastboot-testing.

2. Deleting yarn.lock and removing node_modules results in an error with this package:

   "lint-staged": ">=10",

This package requires node 14, but so far we're only on node 12, and the package will not install. It's used for the husky-based pre-commit hooks.

3. Deleting yarn.lock and removing node_modules also throws the familiar Error: no mixin named -xy-cell-properties sass error (another Foundation UI issue)

4. When going from 2.12 to 2.13, I ran into this — seems related to how babel is dealing with decorators. I think it's also related to caniuse-lite... which I see a lot of warnings about. However, addressing those warnings simply (yarn upgrade) simply gives me the recurring node-sass/foundation css "warning" error (lol) "Error: no mixin named -xy-cell-properties".

5. This deprecated and unmaintained shim for foundation-sites is an enormous source of pain. See what I can do about it .

6. ember-needs-async seems barely updated as far as I can tell. It specifies an ember-data version which causes things to resolve with different versions (which leads to gnarly runtime errors).

[allthesignals]

Landed on 3.13! Automated tests fail for some reason, will fix that soon, but got a quick visual check on things. So, some real skeletons in the closet here...

1. We have an addon, ember-mapbox-composer, that itself overrides some private methods in ember-mapbox-gl. That's bad. But even worse, labs-zola extends ember-mapbox-composer, which also overrides private stuff. So, things were failing because of private internals changed, overrides no longer worked... and I had to add some code to get these to trigger manually.
2. To get around the foundation-sites issue, I had to downgrade and pin the version of foundation-sites due to run-ins with this deprecation warning/error combination issue. The same thing is done in labs-streets
3. ember-needs-async is seemingly abandoned and inserts specific versions of ember-data, causing yarn to resolve with multiple of ember-data. This causes weird inexplicable polyfills to fail/enter the picture. I don't know. Key was to get it to resolve to only one version of ember-data, and that meant forking ember-needs-async and making changes there.

[allthesignals]

3.14 up in PR. Upgrade to 3.15 actually breaks the bookmarks feature. Investigating...

Notes

Ember Data Model Fragments seem to be causing some pain, unsure why... "RecordIdentifier 'zoning-district:C1-5 (@ember-data:lid-zoning-district-C1-5)' to '1', because that id is already in use" and other examples are popping up around the test suite. I can't reproduce manually through the app. Is it mirage?

[allthesignals]

Note that I cannot get Github Actions to run the tests after upgrading to node 12... 🤷

One theory: when I removed Github-hosted packages just to test the Github-hosted packages hypothesis, it caused the tests to fail right out of the gate (https://github.com/NYCPlanning/labs-zola/runs/7229494002?check_suite_focus=true). Re-adding them (NPM-hosted), it still stalls... but I think it's because it's stalling from some WebGL-related reason (the app won't continue loading unless WebGL is available). It's probably just sitting on the first test, waiting for it to finish. Confirmed when removing all acceptance test

[allthesignals]

FYI @TylerMatteo I'm pretty stuck on getting this test suite to run in CI. I'll keep poking around. These tests run and pass locally, naturally. But any tips for debugging CI would be appreciated!

[TylerMatteo]

Hey, I just submitted a review on your PR, sorry for the delay. I'm psyched to see it actually goes all the way to 3.19! I'm afraid I probably won't be much help when it comes to debugging tests but I'm happy to look into it next week.

[TylerMatteo]

Just fyi - once we have your PR approved, I'd like to hold off on merging into develop until we get a small change in related to another project. It should be in early next week. For now, just plan on waiting for me to click the merge button on your PR, even its approved.

[allthesignals]

Hey, I just submitted a review on your PR, sorry for the delay. I'm psyched to see it actually goes all the way to 3.19! I'm afraid I probably won't be much help when it comes to debugging tests but I'm happy to look into it next week.

Great! Sorry all the weird console logs and commenting out things were attempts at debugging GH Actions.

My tests pass locally but since upgrading Node, they simple stall out in CI. I believe this has something to do with webgl not loading in the headless environment, and the "ready" state never being achieved (hence the timeouts).

Once I actually get tests to pass in CI (again, they pass locally 😄), I will remove those junk lines.

Thanks again for the review! I hope to figure out this GH actions stuff asap. Then on to applicant portal backend...

Learnings

So far, all I can glean from this is that some combination of Fastboot, Ember Mapbox Composer, Main Map, and the Bookmarks feature is what's causing tests to stall...

Without the map, it seems to stop at the 3rd bookmark test... with the map, not sure yet... it seems to never get beyond the first test.

Going to see if upgrading ember-qunit will help with the asynchrony isseus: https://github.com/emberjs/ember-qunit/blob/master/docs/migration.md

[TylerMatteo]

Hey Matt, sorry I've been MIA. How's it going with trouble shooting those tests? I saw your email and am happy to schedule something for early next week if you still want to chat.

[allthesignals]

Hi @TylerMatteo no problem. I'm fairly open on Monday if you want to hop on a call then.

[allthesignals]

After a brief call, we settled on:

1. Removing CI for now, asking for tests to be run and verified locally
2. Getting commit-hash-based dependencies pointing to newer versions

[allthesignals]

@TylerMatteo

In this branch, there are two commit-hash-based dependencies. One is for mapbox-composer. I made a PR here that needs a review.

The other is for ember-needs-async which looks to be abandoned. I'll try to PR there but we may not get a new release and might need to fork it to the organization.

[TylerMatteo]

@allthesignals Just took another look at your PR and resolved a bunch of conversations. This is looking pretty close to being ready for a merge into develop to me. The one thing I'd like for us to do is have those commit hash dependencies be pointing to commits in repos owned under the NYCPlanning Github org. I'm out of the office next week but when I'm back we can try to get your ember-mapbox-composer PR (please take a look at my comment on that PR) merged in and publish a version so that we don't need the hash at all. As for ember-needs-async, I can set up a fork of under our GH org when I get back. Almost there! Hopefully you've been able to make some progress on applicant portal backend and aren't totally blocked by your Zola PR languishing a little longer.

[allthesignals]

Thanks @TylerMatteo — sounds good.

FYI I have an applicant protal backend PR up.

[allthesignals]

Okay, two commit hash deps:

• [ ] https://github.com/NYCPlanning/labs-zola/blob/fa8fa4788f767fa80b12867e545a9c05fbf6605b/package.json#L76

Awaiting resolution for this

• [ ] https://github.com/NYCPlanning/labs-zola/blob/fa8fa4788f767fa80b12867e545a9c05fbf6605b/package.json#L82

Will point to org fork when you're back

[TylerMatteo]

Ok I forked ember-needs-async and gave you access so you should be able to push your branch directly if you'd rather do that than PRing from your own fork, up to you. Once we have that merged, you can update the hash here to the one under the NYCPlanning fork

I left a comment on your composer PR as well. Just need to tweak engines and npm version that PR. Once that's merged, I'll publish the new 0.2.5 version to npm so you hopefully won't need the hash for that one.

[TylerMatteo]

@allthesignals Please see my previous comment. Once you update your composer PR with those changes and open a PR against our new ember-needs-async fork, I can see about getting these changes out the door.

[allthesignals]

Thanks @TylerMatteo, been out of on holiday the past week. Will take a look ASAP.

[TylerMatteo]

Hey @allthesignals, how difficult do you think it would be to remove the dependency on ember-needs-async? It looks like it's only used in carto-data-provider and I would love to refactor out an abandoned dependency if we can. Could someone refactor that to just use ember-concurrency directly?

[allthesignals]

@TylerMatteo

I think so... all ember-needs-async does is allow usage of fetch (requests to the store) inside templates. Using ember-concurrency might help there.. or just copying the ember-needs-async code into the repository and updating dependencies accordingly.