-
Notifications
You must be signed in to change notification settings - Fork 0
Source map support #102
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
base: master
Are you sure you want to change the base?
Source map support #102
Conversation
| return nil, err | ||
| } | ||
| _, err = os.Stat(myRootDir + "/" + realDirName + ".js.map") | ||
| if err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this to check if the file exists? If so, I recommend checking fs.ErrNotExist and returning an error if some other err is returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
syspath/code_path.go
Outdated
| servicePathRegexStr = `^code\/services\/([^\/]+)\/([^\/]+)\.(?:js|json)$` | ||
| libraryPathRegexStr = `^code\/libraries\/([^\/]+)\/([^\/]+)\.(?:js|json)$` | ||
| servicePathRegexStr = `^code\/services\/([^\/]+)\/([^\/]+)\.(?:js|json|js.map)$` | ||
| libraryPathRegexStr = `^code\/libraries\/([^\/]+)\/([^\/]+)\.(?:js|json|js.map)$` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to escape the . otherwise it will match any character
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working for me. Do you have an example that breaks this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sanket28 It works for files that ends in js.map, but it will also accept files that end in jsamap, jsbmap, jscmap, and so on...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I got that but do you have a working example that breaks? Like a unit test or Go playground link? I have tested this before with different file extensions and it's working for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func TestRegex(t *testing.T) {
r := `^code\/libraries\/([^\/]+)\/([^\/]+)\.(?:js|json|js.map)$`
re := regexp.MustCompile(r)
matches := re.FindStringSubmatch("code/libraries/service/service.jsamap")
if len(matches) > 0 {
t.Fatalf("Got %d matches, expected 0: %v", len(matches), matches)
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added the escape
| bytsMap, err := ioutil.ReadFile(myRootDir + "/" + realDirName + ".js.map") | ||
| if err != nil { | ||
| fmt.Printf("ioutil.ReadFile for source map failed: %s\n", err) | ||
| return nil, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't the error get printed out already? Won't this result in the error being printed twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a lot of places we just return the error and dont print it at all so its better this way
This adds source map file support to push and pull for services and libraries.