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

serve local file in pxt serve again #8099

Merged
merged 2 commits into from Apr 16, 2021
Merged

serve local file in pxt serve again #8099

merged 2 commits into from Apr 16, 2021

Conversation

@jwunderl
Copy link
Member

@jwunderl jwunderl commented Apr 15, 2021

When I gave #8097 a quick check locally, my local markdown updates weren't going through and I noticed it was fetching everything from the server.

Turns out this weird behavior https://github.com/microsoft/pxt/pull/7989/files#r598994321 of updating as a side effect was the thing preventing it from serving docs from the server instead of the local versions when doing a pxt serve, as setting that to true makes us skip the local request path here:

pxt/pxtlib/emitter/cloud.ts

Lines 184 to 195 in 108e378

if (pxt.BrowserUtils.isLocalHost() && !pxt.Util.liveLocalizationEnabled()) {
return localRequestAsync(url).then(resp => {
if (resp.statusCode == 404)
return privateRequestAsync({ url, method: "GET" })
.then(resp => { return { md: resp.text, etag: <string>resp.headers["etag"] }; });
else return { md: resp.text, etag: undefined };
});
} else {
const headers: pxt.Map<string> = etag && !useCdnApi() ? { "If-None-Match": etag } : undefined;
return apiRequestWithCdnAsync({ url, method: "GET", headers })
.then(resp => { return { md: resp.text, etag: <string>resp.headers["etag"] }; });
}

Fix that so we don't go to makecode.com/api/* for markdown on pxt serve. (the changes are just guarding with pxt.BrowserUtils.isLocalHostDev() in a few places where we don't want to default to fetching from server -- e.g. we still want to get updates when if we're doing incontext translations or if we pass livelang=de)

@jwunderl jwunderl requested review from Jxwoon, livcheerful and shakao Apr 15, 2021
skillmap/src/App.tsx Outdated Show resolved Hide resolved
@shakao
shakao approved these changes Apr 15, 2021
@jwunderl jwunderl merged commit 1902b6f into master Apr 16, 2021
6 checks passed
6 checks passed
@github-actions
Analyze (javascript)
Details
@github-actions
buildpr
Details
@github-actions
buildpush
Details
@github-actions
Analyze (python)
Details
@github-code-scanning
CodeQL 2 analyses not found
Details
@microsoft-cla
license/cla All CLA requirements met.
Details
@jwunderl jwunderl deleted the servelocalfilesagain branch Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants