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

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

Merged
merged 10 commits into from Mar 6, 2019
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -0,0 +1,5 @@
# heading1

content content content

[image](./100x100.png)
33 changes: 33 additions & 0 deletions packages/core/integration-tests/test/markdown.js
@@ -0,0 +1,33 @@
const assert = require('assert');
const path = require('path');
const fs = require('@parcel/fs');
const {bundle, run, assertBundleTree} = require('./utils');

describe('markdown', function() {
it('should support bundling Markdown', async function() {
let b = await bundle(
path.join(__dirname, '/integration/markdown/index.md')
);

await assertBundleTree(b, {
name: 'index.html',
assets: ['index.md'],
childBundles: [
{
type: 'png',
assets: ['100x100.png'],
childBundles: []
}
]
});

let files = await fs.readdir(path.join(__dirname, '/dist'));
let html = await fs.readFile(path.join(__dirname, '/dist/index.html'));
for (let file of files) {
let ext = file.match(/\.([0-9a-z]+)(?:[?#]|$)/i)[0];
if (file !== 'index.html' && ext !== '.map') {
assert(html.includes(file));
}
}
});
});
1 change: 1 addition & 0 deletions packages/core/parcel-bundler/src/Parser.js
Expand Up @@ -51,6 +51,7 @@ class Parser {

this.registerExtension('jade', './assets/PugAsset');
this.registerExtension('pug', './assets/PugAsset');
this.registerExtension('md', './assets/MarkdownAsset');

let extensions = options.extensions || {};
for (let ext in extensions) {
Expand Down
5 changes: 3 additions & 2 deletions packages/core/parcel-bundler/src/Pipeline.js
Expand Up @@ -49,7 +49,6 @@ class Pipeline {

let inputType = path.extname(asset.name).slice(1);
let generated = [];

for (let rendition of this.iterateRenditions(asset)) {
let {type, value} = rendition;
if (typeof value !== 'string' || rendition.final) {
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh, I've got it!

: !(type === 'js' && this.options.scopeHoist)
};
}
}
Expand Down
17 changes: 17 additions & 0 deletions packages/core/parcel-bundler/src/assets/MarkdownAsset.js
@@ -0,0 +1,17 @@
const localRequire = require('../utils/localRequire');
const Asset = require('../Asset');

class MarkdownAsset extends Asset {
constructor(name, options) {
super(name, options);
this.type = 'html';
this.needsPipelineProcessing = true;
this.hmrPageReload = true;
}
async parse(code) {
let marked = await localRequire('marked', this.name);
this.contents = marked(code);
Benjmhart marked this conversation as resolved.
Show resolved Hide resolved
return this.contents;
}
}
module.exports = MarkdownAsset;