Add source.js scopes#83
Merged
Merged
Conversation
bathos
approved these changes
Dec 27, 2019
Owner
bathos
left a comment
There was a problem hiding this comment.
LGTM!
I should mention that when I first wrote stuff, I mainly used .es because I thought that .js was ‘claimed’ by the default def and thought that if I used it, it might create collision problems somewhere. I also wasn’t really aware of most of the things that are sensitive to these suffixes. In practice, it’s absences of .js that have led to interop issues a few times, so I likely made the wrong choice.
I don’t know for sure that dropping .es in favor of having .js only would be safe now, but if you think it would make stuff simpler and that it would be safe, maybe it’d be worth it?
dangh
approved these changes
Dec 27, 2019
Collaborator
dangh
left a comment
There was a problem hiding this comment.
LGTM! Thanks @blake-regalia 🎉
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #82 .