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 package-lock.json version of htmlbars #533

Closed
wants to merge 1 commit into from

Conversation

mansona
Copy link
Member

@mansona mansona commented Aug 18, 2022

I expect this PR to fail 😞

I noticed that some of the ember-try scenarios are failing on my other PR #530 and I couldn't easily see why. So I wrote a little script that methodically stepped through the output of npm outdated and ran npm update on each package until it failed the same test as CI

It turns out that updating ember-cli-htmlbars is causing ember-auto-import to fail for a few of the LTS CI runs.

@mansona
Copy link
Member Author

mansona commented Aug 18, 2022

I took this one step further and did a git bisect on ember-cli-htmlbars to find the commit that failed:

a95a4fedc7a06dc077cd986d426e87205f351e08 is the first bad commit
commit a95a4fedc7a06dc077cd986d426e87205f351e08
Author: Edward Faulkner <edward@eaf4.com>
Date:   Thu Jun 30 01:01:39 2022 -0400

This seems to be the problematic commit: ember-cli/ember-cli-htmlbars@a95a4fe

looking at the diff I guess it's because the current "LTS" tests in ember-auto-import are very very old and before code like this can actually be run: ember-cli/ember-cli-htmlbars@a95a4fe#diff-a164c18a8dda75a288aff21d157c523268f55ecb5f45aafb9786c482d2389916R90-R94

or do you think that we would need to change the order of things so that the output from the transform import { hbs } from 'ember-cli-htmlbars'; can be analysed properly by the splitter?

@mansona
Copy link
Member Author

mansona commented Sep 8, 2022

Update: now that ember-cli/ember-cli-htmlbars#755 is merged this PR should pass because the underlying problems have been solved 🎉

@mansona
Copy link
Member Author

mansona commented Oct 14, 2022

So this doesn't need to be merged any more, it was only illustrating the issue that was blocking #530 from getting merged 👍

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

1 participant