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

Add public import for uniqueId helper #20165

Closed
1 of 3 tasks
Tracked by #871
chriskrycho opened this issue Aug 26, 2022 · 5 comments · Fixed by #20464
Closed
1 of 3 tasks
Tracked by #871

Add public import for uniqueId helper #20165

chriskrycho opened this issue Aug 26, 2022 · 5 comments · Fixed by #20464

Comments

@chriskrycho
Copy link
Contributor

chriskrycho commented Aug 26, 2022

This landed as available via string resolution at unique-id but did not come with a public import, meaning it doesn't currently work in <template>/strict mode. Per discussion at the Framework team meeting a few weeks ago, it should be exported from @ember/helper, the same as the other public helpers which are not built-ins (hash, array, concat, get, and fn etc.).

It needs two things to be public:

Additionally, we'll want to update ecosystem support for it:

@geneukum
Copy link
Contributor

I should be able to take a look at this within a day or two. Should we also add it to the list of 'allowed' built-ins for the no-curly-component-invocation ember-template-lint rule?

@chriskrycho
Copy link
Contributor Author

@geneukum good call. Let me add that to the list of tasks – and also, a colleague mentioned to me late Friday that he’s also interested in helping; let's coordinate efforts in #dev-ember-js on Discord!

@geneukum
Copy link
Contributor

I've opened up the issue: ember-template-lint/ember-template-lint#2592 and PR: ember-template-lint/ember-template-lint#2593 to hopefully get the ember-template-lint part of this handled.

@TechieQian
Copy link
Contributor

TechieQian commented Aug 29, 2022

That colleague is me! That discord channel link doesn't seem to work. Edit: Okay got it.

Question about the re-export, I see we're exposing these functions as part of global Ember object. Should we do that for this as well?

Ember._Input = Input;
Ember._hash = hash;
Ember._array = array;
Ember._concat = concat;
Ember._get = get;
Ember._on = on;
Ember._fn = fn;

mrloop added a commit to mrloop/ember-svg-jar that referenced this issue Nov 28, 2022
The current implementation uses `guidFor` this will cache ids and reuse
them if the same value is passed to `guidFor`, see
https://github.com/emberjs/ember.js/blob/4e3300bdfe75da14f9714b6b1539dbd1612c5af2/packages/%40ember/-internals/utils/lib/guid.ts#L97

The ids in a HTML document should be unique. However if a `title` or
`desc` is reused, for example you have a recurring element, then
`svg-jar` generates ids which match for the recurring element.

This commit uses the `uniqueId` function from the `unqiue-id` helper
to generate a unique id regardless of the svg-jar title or desc matching
previous svg-jar.

Unfortuantely `uniqueId` is not exported so it is copy and pasted.
In the future it will be possible to import it, see
emberjs/ember.js#20165
mrloop added a commit to mrloop/ember-svg-jar that referenced this issue Nov 28, 2022
The current implementation uses `guidFor` this will cache ids and reuse
them if the same value is passed to `guidFor`, see
https://github.com/emberjs/ember.js/blob/4e3300bdfe75da14f9714b6b1539dbd1612c5af2/packages/%40ember/-internals/utils/lib/guid.ts#L97

The ids in a HTML document should be unique. However if a `title` or
`desc` is reused, for example you have a recurring element, then
`svg-jar` generates ids which match for the recurring element.

This commit uses the `uniqueId` function from the `unique-id` helper
to generate a unique id regardless of the `svg-jar` `title` or `desc`
matching previous `svg-jar`.

Unfortunately `uniqueId` is not exported so it is copy and pasted.
In the future it will be possible to import it, see
emberjs/ember.js#20165

Fixes evoactivity#243
@oscareng
Copy link

oscareng commented Jan 5, 2023

does this issue still need fixing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants