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 models length issue #1420

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix models length issue #1420

wants to merge 5 commits into from

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented Sep 30, 2024

A bit difficult to test but in my case I had

<ListSection id="section" dataCollection="gyms" dataFilter="filterGyms" dataTransform="transformGyms" dataFunction="setListData">

which produced:

var models = filterGyms(__alloyId1172);
var len = models.length;

and models was null for me which caused an error "length of undefined".

Instead of crashing it should just use len = 0 for this.

merge #1421 first. There I'm fixing build issues and some other Alloy code

@m1ga
Copy link
Contributor Author

m1ga commented Oct 1, 2024

@cb1kenobi any idea why the tests are failing? They work fine locally

Error:  Titanium configuration file does not exist at "/Users/runner/.titanium/config.json"

not sure how I can fix that.

@cb1kenobi
Copy link
Contributor

cb1kenobi commented Oct 1, 2024

That's not a CLI error, it's an Alloy error:

U.die('Titanium configuration file does not exist at "' + file + '"');

Alloy is reading the config to get the selected SDK, but that's obsolete in CLI v7. Alloy should get the SDK version from the cli object passed into the hook. Alloy should become apart of the CLI/SDK, not some external thing.

@m1ga
Copy link
Contributor Author

m1ga commented Oct 1, 2024

ohhhh ok, I didn't check for the error message in the project 🤦 Let me fix that! Thanks for the hint

@m1ga
Copy link
Contributor Author

m1ga commented Oct 1, 2024

merge #1421 first. There I'm fixing build issues and some other Alloy code

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

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

LGTM. Approving, but waiting for PR #1421 to land.

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.

2 participants