Code snippet hmr, fix #1309

[shigma]

Summary

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Docs
• Build-related changes
• Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

• Chrome
• Firefox
• Safari
• Edge
• IE

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature
• Related documents have been updated
• Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

• support code snippet hmr, fix Not rerender when code snippet file changes #1309
• no more cache on md.render but use cache on md.parse
• cache on md.renderer.render is currently working in progress on this fork (I think it may be better using another pull request)

[shigma]

Something to be discussed before merge

We use cache to speed up md.parse:

const parse = md.parse
const cache = new LRUCache({ max: 1000 })
md.parse = (src, env) => {
  const key = hash(src + env.relPath)
  const cached = cache.get(key)
  if (cached) {
    return cached
  } else {
    const tokens = parse.call(md, src, env)
    cache.set(key, tokens)
    return tokens
  }
}

And these will lead to a problem: if someone changes one of those tokens during render process, it will be a disaster, because changing one of those tokens mean changing cache during render.

Here is an example (markdown/lib/link.js):

-  return Object.assign({}, token, {
-    tag: 'router-link'
+  return Object.create(token, {
+    tag: { value: 'router-link' }
   })

What is the difference? The former use assign, which will drop all the prototype methods (like Token.prototype.attrIndex, and will lead to an error at next time:

const hrefIndex = token.attrIndex('href')
// Error: token.attrIndex is not a function

How can we fix it?

By copy all the tokens before get and set:

md.parse = (src, env) => {
  const key = hash(src + env.relPath)
  const cached = cache.get(key)
  if (cached) {
    return copyTokens(cached)
  } else {
    const tokens = parse.call(md, src, env)
    cache.set(key, copyTokens(tokens))
    return tokens
  }
}

// I actually don't know whether we should
// prevent modification to cached tokens
function copyTokens (tokens) {
  return tokens.map((token) => {
    if (token.children) {
      token.children = copyTokens(token.children)
    }
    return Object.create(token)
  })
}

However, I still don't know whether we should do such thing, because I think users should not change those tokens during rendering process.

[shigma]

@BuptStEve Can you check if this pr fix #1309? And what do you think about the question above?

[BuptStEve]

@BuptStEve Can you check if this pr fix #1309? And what do you think about the question above?

It works.
Just some ideas. Maybe in markdown-loader, we call md.parse to check the cache and we cache the result of md.render?

[shigma]

Just some ideas. Maybe in markdown-loader, we call md.parse to check the cache and we cache the result of md.render?

I think it will not work. Because addDependency was called in the rendering process.

[shigma]

It may be better splitted into some smaller PRs. Closed.