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

macroCondition: do branch elimination if no runtime impl. is involved #1468

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

simonihmig
Copy link
Collaborator

@simonihmig simonihmig commented Jun 8, 2023

This will make macroCondition() do full branch elimination even in development, as long as no macro is involved that has a runtime implementation (get*Config(), isTesting have).

Closes embroider-build/ember-auto-import#582

This will make `macroCondition()` do full branch elimination even in development, as long as no macro is involved that has a runtime implementation (`get*Config()`, `isTesting` have).

Closes #582
applyMode(config);
config.finalize();
});

buildTimeTest('if selects consequent, drops alternate', () => {
test('if selects consequent, drops alternate', () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note how a lot of these buildTimeTest were turned into regular test to make them run in both modes, given that when no runtime macro is involved, they should just behave the same!

@simonihmig simonihmig requested review from ef4 and a team June 8, 2023 16:07
@@ -401,11 +402,11 @@ describe('macroCondition', function () {
expect(code).not.toMatch(/alpha/);
});

runTimeTest('can be used as class field initializer', () => {
runTimeTest('given runtime implementation, it can be used as class field initializer', () => {
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli Jun 8, 2023

Choose a reason for hiding this comment

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

is there a runtime test exercising the possible outcomes of just isTesting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, there is something already here

Copy link
Contributor

@ef4 ef4 left a comment

Choose a reason for hiding this comment

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

Beautiful, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug introduced by #574 when using conditional importSync
3 participants