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

Fix some @import edge cases #890

Merged
merged 2 commits into from Nov 21, 2019

Conversation

@nex3
Copy link
Contributor

nex3 commented Nov 21, 2019

@nex3 nex3 requested a review from jathak Nov 21, 2019
@jathak
jathak approved these changes Nov 21, 2019
for (var modules in _nestedForwardedModules.reversed) {
for (var module in modules.reversed) {
if (module.variables.containsKey(name)) {
module.setVariable(name, value, nodeWithSpan);

This comment has been minimized.

Copy link
@jathak

jathak Nov 21, 2019

Member

I'm a bit confused as to why the module is being mutated here. Wouldn't that allow files with nested imports to make changes to a module that would still be reflected if the module is loaded later elsewhere? Or is that the intended behavior?

This comment has been minimized.

Copy link
@nex3

nex3 Nov 21, 2019

Author Contributor

Yes, this is intended. If you @import a forwarded library, you should be able to set that library's variables (just like you'd be able to if you @used it, or if you @importeed a library that uses @import instead of @forward). See for example this spec.

@nex3 nex3 merged commit 9635a52 into master Nov 21, 2019
3 checks passed
3 checks passed
cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nex3 nex3 deleted the import-forward-twice branch Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.