Skip to content

remove the offline manifest authoring code#237

Draft
scytacki wants to merge 1 commit intooffline-mode-devfrom
remove-authoring-mode-code
Draft

remove the offline manifest authoring code#237
scytacki wants to merge 1 commit intooffline-mode-devfrom
remove-authoring-mode-code

Conversation

@scytacki
Copy link
Member

@scytacki scytacki commented Apr 8, 2021

Since we are generating the manifests statically, and this authoring code doesn't currently work with the CacheOnly Strategy used by the app, I think it make sense to remove it.

It might also help to reduce merge conflicts when the merging in the master branch.

Perhaps we'll want to bring this back if we can't statically handle
new activities, the README references a tag to indicate the last commit when it was working.

TODO

  • update the README with more info about the static generation

We now can generate the manifests statically.
Additionally this code didn't work after the update to using an install page.
With a default CacheOnly Strategy.
Perhaps we'll want to bring this back if we can't statically handle
new activities, but it seems best to remove it since it doesn't currently work.
@scytacki
Copy link
Member Author

scytacki commented Apr 8, 2021

@dougmartin I'm curious what you think about removing this. I know it was your baby.

I doubt that static analysis is going to work for everything, but relying on manual navigation through the activity was error prone.

My main goal is to try to clean things up so when we come back to this in a few months (or years?) things are less confusing.

@dougmartin
Copy link
Member

@scytacki I really don't care if it is removed but I'm just wondering how we discover the runtime assets like the iframe app Javascript and css without this mode.

@scytacki
Copy link
Member Author

scytacki commented Apr 8, 2021

The script currently finds them by downloading the html files and looking for script and link tags. It also looks for some NetLogo specific code:

export const getAssetsInHtml = (htmlString: string): string[] => {
// <script src="../video-player/assets/index.4625e4716a2dfe8f0462.js"
const scriptAssets = matchAllFirstGroup(htmlString, /<script src="([^"]*)"/g);
// import-drawing "./ak-base-map-with-rose.png"
// Note the use of the m flag so we are using multi-line mode
const netlogoImports = matchAllFirstGroup(htmlString, /^\s*import-drawing\s+"([^"]*)"/gm);
// fetch:url-async ("https://s3.amazonaws.com/cc-project-resources/precipitatingchange/images-2021/ak-w-cities.png")
// Note the use of the m flag so we are using multi-line mode
const netlogoFetches = matchAllFirstGroup(htmlString, /^\s*fetch:url-async\s*\(\s*"([^"]*)"/gm);
return scriptAssets.concat(netlogoImports).concat(netlogoFetches);
};
export const getCssLinksInHtml = (htmlString: string): string[] => {
// <link href="../video-player/assets/index.4625e4716a2dfe8f0462.css"
// <link href="https://fonts.googleapis.com/css?family=Lato"
return matchAllFirstGroup(htmlString, /<link href="([^"]*)"/g);
};

@scytacki
Copy link
Member Author

scytacki commented Apr 8, 2021

It even goes as far as looking for url( patterns in the css referenced by the index file:

for (const cssLink of uniqueCssLinksToFetch) {
console.log("fetching: ", cssLink);
const response = await fetch(cssLink);
const body = await response.text();
// match url(https://fonts.gstatic.com/s/lato/v17/S6uyw4BMUTPHjx4wXiWtFCc.woff2)
// NOTE there are many other types of urls that we aren't matching here
const cssUrls = matchAllFirstGroup(body, /url\((https:\/\/[^)'"]*)\)/g);
// NOTE we are only matching absolute urls so we don't need to resolve them
urls.push(...cssUrls);
}

All of that code doesn't cover all of the cases though. And might be hard to maintain because a fix for one set of activities might cause another set of activities to start pulling down extraneous stuff that it doesn't need. But at least in this first case of the PC content it seemed better to figure out those heuristics rather than running through the activity pages in a browser.

If we run into a problem of fixing one thing breaks another, perhaps we can add per-manifest configuration options to turn on and off various static analysis patterns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants