Skip to content

fix: ensure ember-data works properly #119

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

Merged
merged 4 commits into from
Apr 10, 2023

Conversation

runspired
Copy link
Contributor

@runspired runspired commented Mar 31, 2023

  • enable local builds of ember-data to be used
  • fixes for ember-data docs not appearing properly

Docs Generation Changes

resolves #117
partially resolves #118 by enabling work arounds

note, with this change you must explicitly opt-in to install and build of docs locally.

yarn gen --project ember --version 4.12.0 --build --install

If you don't opt in you will get whatever docs build was last built in that project, which for local purposes like this is typically preferred anyway.

Docs Parsing Changes

Updated Parser

Updated the parser to resolve several issues with EmberData docs not being correctly parsed. I've manually reviewed all EmberData documentation and this has no ill-effects. We should also review the Ember documentation.

  • updated highlight.js from ^10.4.1 to ^11.7.0
  • updated marked from ^0.7.0 to ^4.3.0

Fixed Async Issues

  • the generate-local command now directly invokes main vs running yarn via execa. This ensures that crashes are correctly reported, they weren't before.
  • various promise spaghetti has been refactored to async/await and a few things that could race before were made linear (the for ... of change). This generally enabled errors to surface instead of getting lost. We also seem to always complete the docs build now, before if an error occurred we would see "success" but not actual have an updated asset or see an error.

Fixed Bug

There was a bug in which documentation with an id longer than 50 would not be saved into the json-docs.

if (document.data.id.length > 50) {
// wtf ember 1.0 docs??
return resolve()

This led to the module @ember-data/experimental-preview-types not being built properly. I've utilized the code comment to improve the heuristic to capture whatever case this was originally here for (sounds like no one knows?) so that EmberData's docs will correctly build.

@runspired runspired changed the title fix: enable local builds of ember-data to be used fix: ensure ember-data works properly Mar 31, 2023
Copy link
Contributor

@jenweber jenweber left a comment

Choose a reason for hiding this comment

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

Jared and I tested this locally for a couple Ember versions, and it seems good to go!

@runspired we ran into one issue, which is that the data repo has very specific pnpm requirements. Could that be changed to allow the specified version or any later version within the major, i.e. >= ^8.1.0 or >= ~8.1.0? Currently, it seems to require exactly the version down to the patch digit. Our docs release process has quite a few hurdles in it, so if we can reduce this one, that will be helpful.

Note to learning team contributors: This change breaks our release tool due to the pnpm change above and the install flag, but we were already usually running these releases manually lately. One solution is that we could make a script for package.json that handles these 2 issues, and use the new command in the release tool instead of what we have in there currently.

@jenweber jenweber merged commit 56ea9a5 into ember-learn:master Apr 10, 2023
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.

local docs generation doesn't have permission to execute scripts yarn gen command does not work with ember-data
2 participants