Add Markdown support as an entry point (issue #2274) #2538

Benjmhart · 2019-01-13T18:19:46Z

↪️ Pull Request

added support for markdown entry points with dynamic installation of the marked package to parse the markdown code into html.
added initial test for markdown parsing
this is a partial fix toward ticket Markdown Support #2274
Markdown Support #2274

💻 Examples

user can now run parcel build index.md, and get an index.html in the /dist folder

🚨 Test instructions

unit test is provided in the integration tests. it confirms that the markdown file is parsed into html, and that image assets are registered/handled correctly.

✔️ PR Todo

Added/updated unit tests for this change
Filled out test instructions (In case there aren't any unit tests)
Included links to related issues/PRs

devongovett · 2019-01-16T19:57:46Z

packages/core/parcel-bundler/src/assets/MarkdownAsset.js

+const localRequire = require('../utils/localRequire');
+const HTMLAsset = require('./HTMLAsset');
+
+class MarkdownAsset extends HTMLAsset {


I wouldn't recommend extending from another asset type. Instead, we can rely on the pipeline to pass through the generated HTML to the HTMLAsset.

Extend from Asset here, and return the html from the generate method. For an example, see how e.g. JSONAsset does it.

Rather the PugAsset:

parcel/packages/core/parcel-bundler/src/assets/PugAsset.js

Line 8 in d7dc51f

this.type = 'html';

parcel/packages/core/parcel-bundler/src/assets/PugAsset.js

Line 36 in d7dc51f

return compiled(config.locals);

thanks, I'll take a look this weekend and also see about resolving the pipeline issues

Benjmhart · 2019-01-20T19:06:00Z

Hi there,

So we've adjusted the MarkdownAsset to avoid extending the HTML Asset. We did need to add a flag to make the pipeline behave properly. Thank you for your useful examples in the JS and pug assets @devongovett and @mischnic

Please let us know if this is more in line with what you were looking for.

Also, we wanted to discuss front matter support. While the marked library appears to include support for basic front matter (mainly styling). the original feature request included mustache/templating specs. We held off on this aspect of the feature because it makes parcel a bit opinionated on exactly how it should be done, and we wanted input from the core team before moving ahead there.

Thanks

devongovett · 2019-01-21T03:45:24Z

packages/core/parcel-bundler/src/Pipeline.js

@@ -117,7 +116,9 @@ class Pipeline {
        type,
        value: asset.generated[type],
        // for scope hoisting, we need to post process all JS
-        final: !(type === 'js' && this.options.scopeHoist)
+        final: asset.hasOwnProperty('needsPipelineProcessing')
+          ? !asset.needsPipelineProcessing


Why is this new property needed? Normally, if the type of the asset changes (e.g. from .md to .html), the pipeline will automatically pass it through to the next asset type (in this case HTMLAsset).

Thanks for your comment!

Normally, if the type of the asset changes (e.g. from .md to .html), the pipeline will automatically pass it through to the next asset type

Could you, please point out where that happens exactly?

From what we discovered, the only way for an asset to undergo one more processing by another asset type is for this final property to be false.

parcel/packages/core/parcel-bundler/src/Pipeline.js

Lines 55 to 58 in bdc044a

if (typeof value !== 'string' || rendition.final) {

generated.push(rendition);

continue;

}

As you can see in the diff, the original code only allows final to be false for JS.

Maybe the right way to go would be to remove those 4 lines above altogether. 😱 But I suppose they are there for a reason.

Please, let me know if I've got it wrong.

Ohhh, I've got it!

packages/core/parcel-bundler/src/assets/MarkdownAsset.js

kirillrogovoy · 2019-01-25T11:23:31Z

Hey, @devongovett!

Just a quick update.

Our plan:

Wait for you to take another look, and hopefully approve this PR.
Get it merged to the upstream. (Wow, we have basic but useless support for Markdown!)
Get your feedback on how the templating should work (Add Markdown support as an entry point (issue #2274) #2538 (comment)).
Implement whatever we agree on in a separate PR.

Sounds good?

kirillrogovoy · 2019-02-28T07:24:45Z

Shamelessly pinging you guys! 🙂
@devongovett @mischnic

tomfinney · 2019-03-30T09:30:23Z

Is there a way to opt out of this or something? I was importing markdown files with a glob import like

import foo from "/markdown/*.md";
// {
//   'file-1': '/file-1.8e73c985.md',
//   'file-2': '/file-1.8e73c985.md'
// }

Now, instead of file paths for each markdown file, I get the entire contents of the markdown file parsed as HTML which is cool but the meta data for each md file is now in the HTML body.

kirillrogovoy · 2019-03-31T08:37:54Z

Hey @tomfinney,

Thanks for reporting this.

I'll try to look into this soon.

Could you, please describe the sort of meta data you used and which is now inaccessible?
The old and new behaviors aside, what would be the perfect interface for your use case?

tomfinney · 2019-03-31T09:23:19Z

Hi @kirillrogovoy

I was using the YAML front matter style to prepend markdown files with some meta data based on: https://jekyllrb.com/docs/front-matter/

I have an example of one of my files here: https://github.com/tomfinney/website/blob/master/src/markdown/blogs/01_new_website.md

I'm not really sure if using the YAML style for storing meta data in a file is standard so I guess it would make sense that it would be parsed as HTML. It's just now when I import a markdown file, I have no way of splitting the metadata from the contents unless I add some special delimiter characters around them.

Originally, I was fetching the contents of the markdown file and the splitting on the --- and using a YAML parser to convert the meta data to an object.

So I guess the perfect interface for my use case would be that if I were to use a glob import, it would still return the keyed object of the file name to the file paths so I can choose how to process the files.

kirillrogovoy · 2019-03-31T09:33:13Z

I see.

Would it work for you if import resulted in a keyed object containing the parsed metadata and the parsed HTML?

I'm planning on adding the front-matter support out of the box.

tomfinney · 2019-03-31T09:36:10Z

I was just writing another comment about how that would actually be the best solution. I'm essentially just replicating front matter at the moment so if that was supported out of the box, it'd be perfect.

devongovett requested changes Jan 16, 2019

View reviewed changes

kirillrogovoy and others added 6 commits January 20, 2019 10:25

WIP

1cf46d7

added marked

b8b4b47

Add MarkdownAsset

99ca23d

added dynamic install of marked, + DRYed parse method

7c0d812

Add tests

ed6c6cf

it works goshdanggit

e5a00e7

Benjmhart force-pushed the master branch from 6a03cb4 to e5a00e7 Compare January 20, 2019 18:13

add needsPipelineProcessing to the Asset

6c78aca

devongovett reviewed Jan 21, 2019

View reviewed changes

packages/core/parcel-bundler/src/assets/MarkdownAsset.js Outdated Show resolved Hide resolved

Remove needsPipelineProcessing

199fe18

Benjmhart and others added 2 commits January 31, 2019 20:37

Merge branch 'master' into master

22cdea0

Merge branch 'master' into master

93b83be

devongovett approved these changes Mar 6, 2019

View reviewed changes

devongovett merged commit 03fdcfd into parcel-bundler:master Mar 6, 2019

mischnic mentioned this pull request Mar 18, 2019

Markdown Support #2274

Closed

bbugh mentioned this pull request Mar 23, 2019

Markdown entry point with relative path producing odd links #2835

Closed

kirillrogovoy mentioned this pull request Apr 19, 2019

Add Markdown support — v2 #2882

Closed

3 tasks

lol768 mentioned this pull request Aug 21, 2019

Parcel 2: Expected constructor errors when building or watching #3450

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Markdown support as an entry point (issue #2274) #2538

Add Markdown support as an entry point (issue #2274) #2538

Benjmhart commented Jan 13, 2019

devongovett Jan 16, 2019

mischnic Jan 16, 2019

Benjmhart Jan 19, 2019

Benjmhart commented Jan 20, 2019

devongovett Jan 21, 2019

kirillrogovoy Jan 21, 2019

kirillrogovoy Jan 21, 2019

kirillrogovoy commented Jan 25, 2019

kirillrogovoy commented Feb 28, 2019

tomfinney commented Mar 30, 2019 •

edited

kirillrogovoy commented Mar 31, 2019

tomfinney commented Mar 31, 2019

kirillrogovoy commented Mar 31, 2019

tomfinney commented Mar 31, 2019

	if (typeof value !== 'string' \|\| rendition.final) {
	generated.push(rendition);
	continue;
	}

Add Markdown support as an entry point (issue #2274) #2538

Add Markdown support as an entry point (issue #2274) #2538

Conversation

Benjmhart commented Jan 13, 2019

↪️ Pull Request

💻 Examples

🚨 Test instructions

✔️ PR Todo

devongovett Jan 16, 2019

Choose a reason for hiding this comment

mischnic Jan 16, 2019

Choose a reason for hiding this comment

Benjmhart Jan 19, 2019

Choose a reason for hiding this comment

Benjmhart commented Jan 20, 2019

devongovett Jan 21, 2019

Choose a reason for hiding this comment

kirillrogovoy Jan 21, 2019

Choose a reason for hiding this comment

kirillrogovoy Jan 21, 2019

Choose a reason for hiding this comment

kirillrogovoy commented Jan 25, 2019

kirillrogovoy commented Feb 28, 2019

tomfinney commented Mar 30, 2019 • edited

kirillrogovoy commented Mar 31, 2019

tomfinney commented Mar 31, 2019

kirillrogovoy commented Mar 31, 2019

tomfinney commented Mar 31, 2019

tomfinney commented Mar 30, 2019 •

edited