fix link to work with global rule #666
Conversation
| @@ -155,7 +155,9 @@ export default class RuleList { | |||
| const cssRule = cssRules[i] | |||
| let key = this.options.sheet.renderer.getKey(cssRule) | |||
| if (map[key]) key = map[key] | |||
| const rule = this.map[key] | |||
|
|
|||
| const mappedGlobal: any = this.map['@global'] | |||
There was a problem hiding this comment.
core doesn't know anything about plugins, it needs to be solved locally by a plugin, if plugin is missing what it needs, core should provide a generic solution so that plugin can fix it
There was a problem hiding this comment.
@kof Thats what I was thinking that this should be moved to the plugin itself. On this weekend if I get some time I will try to move it there. as of right now I am not sure how the plugin should handle this.
There was a problem hiding this comment.
hey @sambhav-gore I see you got distracted, any chance you can finish this one?
|
@sambhav-gore any news on this ? |
|
Are there any updates on this? @sambhav-gore |
|
@kof is this still relevant? We need this to work for a long time now, should we switch to an alternative? |
|
linking logic has been largely refactored, I need to check if there is still an issue in the next version. note master is in the preparation for v10
… On 22. Oct 2018, at 14:48, Zadkiel ***@***.***> wrote:
@kof <https://github.com/kof> is this still relevant? We need this to work for a long time now, should we switch to an alternative?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#666 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADOWG6QsxtQgcwPMR2jXqS2ugKPN_qPks5unb6zgaJpZM4Rj1yT>.
|
|
@kof let's maybe close this pr and add a test for it maybe? |
|
Lets add the tests first to see if the problem still exists. |
|
Seems we already have tests and they are passing: https://github.com/cssinjs/jss/blob/master/packages/jss-plugin-syntax-rule-value-function/src/plugin-global.test.js |
|
seems like that, yes, @sambhav-gore wanna try the new version directly from master and see if your use case is fixed, since you didn't provide a test, we can't be 100% sure, but it seems like we have this covered and it works. |
|
Closing as this seems to be fixed now. |
Related to #664
As I was also trying to get it work I fixed this.
This works for me and all tests pass, but please check if this causes any side effects.
May be this should be moved to the jss-global plugin ? ( I am not sure about it), I had to use the 'any' type as the actual type of the rule is 'GlobalContainerRule' which is part of jss-global. Let me know if I should improve this PR or you can adapt it.