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

Try to better catch failures to used aliased names in unit tests #465

Closed
Conduitry opened this issue Apr 11, 2017 · 3 comments
Closed

Try to better catch failures to used aliased names in unit tests #465

Conduitry opened this issue Apr 11, 2017 · 3 comments

Comments

@Conduitry
Copy link
Member

I'm not sure exactly what this would look like. But it would be nice if unit tests were better at catching things like using component directly when you should be using ${block.component} because it may have been renamed. My initial idea is to, during unit tests, make the get unique name methods add some random string to each name they generate, to try to break code that's failing to use the proper alias.

@Conduitry
Copy link
Member Author

diff --git a/src/generators/Generator.js b/src/generators/Generator.js
index 272e843..b89da12 100644
--- a/src/generators/Generator.js
+++ b/src/generators/Generator.js
@@ -238,6 +238,7 @@ export default class Generator {
        }

        getUniqueName ( name ) {
+               name += '_BREAK_SHIT';
                let alias = name;
                for ( let i = 1; reservedNames.has( alias ) || this.importedNames.has( alias ) || this._usedNames.has( alias ); alias = `${name}_${i++}` );
                this._usedNames.add( alias );
@@ -247,6 +248,7 @@ export default class Generator {
        getUniqueNameMaker ( params ) {
                const localUsedNames = new Set( params );
                return name => {
+                       name += '_BREAK_SHIT';
                        let alias = name;
                        for ( let i = 1; reservedNames.has( alias ) || this.importedNames.has( alias ) || this._usedNames.has( alias ) || localUsedNames.has( alias ); alias = `${name}_${i++}` );
                        localUsedNames.add( alias );

Ran unit tests with this just as a quick little test. As advertised it does indeed break shit, but some may be false positives.

@Rich-Harris
Copy link
Member

Excellent point, thanks for being vigilant about this stuff. I seem to have a blind spot for it 😀 Started work over on #468

Rich-Harris added a commit that referenced this issue Apr 11, 2017
Rich-Harris added a commit that referenced this issue Apr 11, 2017
[WIP] catch hardcoded names that should be aliases
@Rich-Harris
Copy link
Member

Will close this as I think we're in a pretty good place now

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

No branches or pull requests

2 participants