Skip to content

Set file.result when processing to non-text#87

Merged
wooorm merged 1 commit intomasterfrom
non-serialized-compilation
Mar 29, 2020
Merged

Set file.result when processing to non-text#87
wooorm merged 1 commit intomasterfrom
non-serialized-compilation

Conversation

@wooorm
Copy link
Copy Markdown
Member

@wooorm wooorm commented Mar 26, 2020

unified is typically, but not always, used to serialize when it compiles. Compilers don’t always return such a serialized (string, Buffer) though. The previous behavior was to place the result of the stringify phase when processing on file.contents. This doesn’t make sense for non-text. This change places non-text results on file.result.

Closes vfile/vfile#45.

unified is typically, but not always, used to *serialize* when it
compiles.  Compilers don’t always return such a serialized (`string`,
`Buffer`) though.  The previous behavior was to place the result of
the stringify phase when processing on `file.contents`.  This doesn’t
make sense for non-text.  This change places non-text results on
`file.result`.

Closes vfile/vfile#45.
@wooorm wooorm added 🦋 type/enhancement This is great to have 🧑 semver/major This is a change 🗄 area/interface This affects the public interface 🙆 yes/confirmed This is confirmed and ready to be worked on labels Mar 26, 2020
@wooorm wooorm requested a review from a team March 26, 2020 10:14
@wooorm wooorm self-assigned this Mar 26, 2020
@wooorm
Copy link
Copy Markdown
Member Author

wooorm commented Mar 26, 2020

checks if the result is string, Buffer, or nully, and based on that either sets vfile.contents or vfile.result

This may work.
Thinking of edge cases

  1. what if the return type is a class instance which implements a custom toString?
class Example {
  toString() {
    return "<h1>hello world</h1>";
  }
}

// where the compiler returns
new Example();

// which when used as a string
`${new Example()}`
// logs "<h1>hello world</h1>"

Is this something that would be considered a value or a result?

  1. Depending on how you interpret "nully", would that include falsy values? Like number with 0, boolean with false, would those be split? (E.G. 0 would go to vfile.contents, 1 to vfile.result)

@ChristianMurphy


Not handled differently. It’s a non-text (neither string nor buffer), so it goes into result. So, if some vdom node was returned that has a toString field or function in its __proto__, those will go into result.

Nully values are ignored (so a compiler could signal that the current file.value or file.result are not to be modified), string and buffer are new values for file.contents, and other values all go into file.result.

wooorm added a commit to unifiedjs/unified-engine that referenced this pull request Mar 26, 2020
wooorm added a commit to unifiedjs/unified-engine that referenced this pull request Mar 26, 2020
Related to vfile/vfile#45.
Related to unifiedjs/unified#87.
Closes GH-45.

Reviewed-by: Christian Murphy <christian.murphy.42@gmail.com>
@wooorm wooorm merged commit c3ba172 into master Mar 29, 2020
@wooorm wooorm deleted the non-serialized-compilation branch March 29, 2020 17:18
@wooorm wooorm added ⛵️ status/released and removed 🙆 yes/confirmed This is confirmed and ready to be worked on labels Mar 30, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧑 semver/major This is a change 🦋 type/enhancement This is great to have

Development

Successfully merging this pull request may close these issues.

Make VFileContents generic to support processors that return objects

1 participant