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

Before 1253 #1256

Closed
wants to merge 4 commits into from
Closed

Before 1253 #1256

wants to merge 4 commits into from

Conversation

jamescdavis
Copy link
Contributor

@jamescdavis jamescdavis commented Jan 26, 2021

This reverts #1253, but adds back it's tests, plus the tests from #1255 and dfreeman#1 to see what will happen. 😃

customizeComponentName does not cause components to conflict with existing symbols fails (as expected since that's what #1253 was addressing) but the others pass. I've marked the failing test as a todo.

@rwjblue
Copy link
Member

rwjblue commented Jan 26, 2021

not ok 221 Chrome 87.0 - [2 ms] - integration - [glimmer-compiler] precompile: customizeComponentName does not cause components to conflict with existing symbols
    ---
        actual: >
            null
        stack: >
            Error: component name is a free variable lookup
                at debugAssert (http://localhost:7357/147658231658/tests/assets/glimmer-vm.js:39626:13)
                at Object.<anonymous> (http://localhost:7357/147658231658/tests/assets/tests.js:3741:24)
                at runTest (http://localhost:7357/147658231658/tests/assets/qunit.js:3048:30)
                at Test.run (http://localhost:7357/147658231658/tests/assets/qunit.js:3034:6)
                at http://localhost:7357/147658231658/tests/assets/qunit.js:3265:12
                at processTaskQueue (http://localhost:7357/147658231658/tests/assets/qunit.js:2621:24)
                at http://localhost:7357/147658231658/tests/assets/qunit.js:2625:8
        message: >
            Died on test #1     at http://localhost:7357/147658231658/tests/assets/tests.js:3728:5
                at Object.<anonymous> (http://localhost:7357/147658231658/tests/assets/tests.js:16086:7)
                at processModule (http://localhost:7357/147658231658/tests/assets/qunit.js:1200:16)
                at Object.module$1 [as module] (http://localhost:7357/147658231658/tests/assets/qunit.js:1225:4)
                at module (http://localhost:7357/147658231658/tests/assets/tests.js:16085:18)
                at Module.callback (http://localhost:7357/147658231658/tests/assets/tests.js:3702:23)
                at Module.exports (http://localhost:7357/147658231658/tests/assets/loader.js:106:32)
                at requireModule (http://localhost:7357/147658231658/tests/assets/loader.js:27:18): component name is a free variable lookup
        negative: >
            false
        browser log: |

@jamescdavis
Copy link
Contributor Author

Yeah, wasn't that simple. Did this in a last-minute-of-the-day rush. Will look at it again later.

@jamescdavis
Copy link
Contributor Author

#1255 fixed the issues covered in these tests without reverting

@jamescdavis jamescdavis closed this Feb 1, 2021
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.

2 participants