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

Incorrect selectors for querySelectorAll #48

Closed
nin-jin opened this issue Nov 29, 2019 · 12 comments · Fixed by #49
Closed

Incorrect selectors for querySelectorAll #48

nin-jin opened this issue Nov 29, 2019 · 12 comments · Fixed by #49

Comments

@nin-jin
Copy link
Contributor

nin-jin commented Nov 29, 2019

Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Document': '#$mol_app_todomvc.Root(0).Title()' is not a valid selector.

App: https://mol.js.org/app/todomvc/

You should use getElementById:

document.getElementById('$mol_app_todomvc.Root(0).Task_row(1).Title()')

Or use right escaping:

document.querySelector('#\\$mol_app_todomvc\\.Root\\(0\\)\\.Task_row\\(1\\)\\.Title\\(\\)')
@dsheiko
Copy link
Owner

dsheiko commented Nov 29, 2019

Thanks, a nice one. It gets included in v3.0.1

@nin-jin
Copy link
Contributor Author

nin-jin commented Nov 29, 2019

When will it be?

@dsheiko
Copy link
Owner

dsheiko commented Nov 29, 2019

I believe in a week, want to collect other possible issues for the release

@nin-jin
Copy link
Contributor Author

nin-jin commented Dec 5, 2019

I really want to try puppetry. But I can't until this fix are released.

@dsheiko
Copy link
Owner

dsheiko commented Dec 5, 2019

But as I understand, you built Puppetry when testing the fix. So you can run it like "npm start". I need extra checks before releasing the last changes.
You can also build it on your own with:
npm run dist:all
it creates dist folder with builds per platform

@nin-jin
Copy link
Contributor Author

nin-jin commented Dec 5, 2019

But as I understand, you built Puppetry when testing the fix.

Nope. :-)

@dsheiko
Copy link
Owner

dsheiko commented Dec 5, 2019

You can play with dev version https://we.tl/t-jyrOwDnXGu

@nin-jin
Copy link
Contributor Author

nin-jin commented Dec 5, 2019

Yeah, it works. But could you make custom names for selectors are optional? It completely useless for me, because ids is already readable.

image

@dsheiko
Copy link
Owner

dsheiko commented Dec 6, 2019

Oops, didn't get it. Recorder tries to suggest a passing name automatically. Optionally you can right-click on a target and specify a custom name.

@nin-jin
Copy link
Contributor Author

nin-jin commented Dec 6, 2019

I don't want to name selectors (so for assertion I have no choice), I want to use selector itself (but without escaping of course).

@dsheiko
Copy link
Owner

dsheiko commented Dec 6, 2019

In your particular case it's so, but usually the selectors are like "div > div:nth-child(2) > div:nth-child(2) > div:nth-child(2) > div:nth-child(2)". Too long for searching in select and meaningless. You can rename targets as you wish. When editing as CSV (as a text) you can use any tools for bulk renaming.

@nin-jin
Copy link
Contributor Author

nin-jin commented Dec 9, 2019

"div > div:nth-child(2) > div:nth-child(2) > div:nth-child(2) > div:nth-child(2)" is too fragile selectors. So in my architecture all elements have user-readable stable semantic id's. I don't want fragile selectors. So I don't want to name or rename selectors. I want selectors naming to be totally optional.

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 a pull request may close this issue.

2 participants