Skip to content
This repository was archived by the owner on Jul 13, 2020. It is now read-only.

Restrict scopes of __moduleName #410

Merged
merged 1 commit into from
Jul 9, 2015
Merged

Restrict scopes of __moduleName #410

merged 1 commit into from
Jul 9, 2015

Conversation

toddwong
Copy link

@toddwong toddwong commented Jul 8, 2015

// file foo.js
import bar from "bar"
console.log(__moduleName);

Without this patch, the code above will output "bar" instead of "foo" if this is the first time "bar" was imported.

@guybedford
Copy link
Member

Nice catch, thanks. Let's just make it a function argument then instead of a var?

@@ -24,7 +24,7 @@ var transpile = (function() {
transpileFunction = babelTranspile;

// note __moduleName will be part of the transformer meta in future when we have the spec for this
return 'var __moduleName = "' + load.name + '";' + transpileFunction.call(self, load, transpiler) + '\n//# sourceURL=' + load.address + '!transpiled';
return '(function(){var __moduleName = "' + load.name + '";' + transpileFunction.call(self, load, transpiler) + '\n})();//# sourceURL=' + load.address + '!transpiled';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline after the })(); so the source map comment is on a fresh line.

@toddwong
Copy link
Author

toddwong commented Jul 9, 2015

Thanks guybedford!

Yes, function argument is better!
Move the newline to after the })(); fails some tests (maybe because the result of transpileFunction.call(self, load, transpiler) ends with a comment line?), so it may be better to add anothing newline after the })(); instead?

I don't know the GitHub thing very well, should I attach another patch to this PR? or just submit another PR? Or maybe you could just do that for us directly and close this PR? (And I do think the last option is more efficient :) )

@guybedford
Copy link
Member

Ah yes, two newlines then perhaps?

You can just add a new commit to the branch and it will show up on this PR. You can even rebase and force push the same branch to keep it as a single commit.

@toddwong
Copy link
Author

toddwong commented Jul 9, 2015

Done! Easier than I thought.

@guybedford
Copy link
Member

Thanks!

guybedford added a commit that referenced this pull request Jul 9, 2015
Restrict scopes of __moduleName
@guybedford guybedford merged commit 686823d into ModuleLoader:master Jul 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants