fix: parse ::directives correctly (#41186)

tools/challenge-parser/parser/plugins/replace-imports.js

@@ -31,6 +31,10 @@ function plugin() {
     }
     const importPromises = importedFiles.map(async ({ attributes }) => {
       const { from, component } = attributes;
+      // if these are missing, bail, since it's not an import.
+      if (!from || !component) {
+        return null;
+      }
       const location = path.resolve(file.dirname, from);
       return await read(location)
         .then(parse)
@@ -65,13 +69,27 @@ function plugin() {
     // Also, we remove the import statements here.
     Promise.all(importPromises)
       .then(() => {
-        remove(tree, { type: 'leafDirective', name: 'import' });
+        remove(tree, isImportNode);
         next();
       })
-      .catch(next);
+      .catch(err => {
+        console.error('error processing ::import');
+        console.error(err);
+        next(err);
+      });
   }
 }
 
+function isImportNode({ type, name, attributes }) {
+  if (!attributes) return false;
+  return (
+    type === 'leafDirective' &&
+    name === 'import' &&
+    attributes.component &&
+    attributes.from
+  );
+}
+
 function validateImports(fileTree) {
   let valid = true;
 

tools/challenge-parser/parser/plugins/replace-imports.test.js

@@ -1,4 +1,4 @@
-/* global describe it expect */
+/* global describe it expect jest */
 const path = require('path');
 const cloneDeep = require('lodash/cloneDeep');
 const toVfile = require('to-vfile');
@@ -7,12 +7,14 @@ const selectAll = require('unist-util-select').selectAll;
 const addImports = require('./replace-imports');
 const originalImportsAST = require('../__fixtures__/ast-imports.json');
 const originalImportsTwoAST = require('../__fixtures__/ast-imports-two.json');
+const originalImportsExtraAST = require('../__fixtures__/ast-imports-extra.json');
 const originalSimpleAST = require('../__fixtures__/ast-simple.json');
 const originalMarkerAST = require('../__fixtures__/ast-marker-imports.json');
 
 describe('replace-imports', () => {
   let importsAST;
   let importsTwoAST;
+  let importsExtraAST;
   let simpleAST;
   let markerAST;
   let correctFile;
@@ -21,6 +23,7 @@ describe('replace-imports', () => {
   beforeEach(() => {
     importsAST = cloneDeep(originalImportsAST);
     importsTwoAST = cloneDeep(originalImportsTwoAST);
+    importsExtraAST = cloneDeep(originalImportsExtraAST);
     simpleAST = cloneDeep(originalSimpleAST);
     markerAST = cloneDeep(originalMarkerAST);
     correctFile = toVfile(
@@ -56,12 +59,15 @@ describe('replace-imports', () => {
   });
 
   it('should fail when the imported file cannot be found', done => {
+    expect.assertions(1);
+    console.error = jest.fn();
     const plugin = addImports();
 
     // we have to rely on the next callback, because that is how you get error
     // messages out of transformers
     const next = err => {
       if (err) {
+        expect(console.error).toHaveBeenCalledTimes(2);
         done();
       } else {
         done('An error should have been thrown by addImports');
@@ -118,6 +124,26 @@ describe('replace-imports', () => {
     plugin(importsAST, correctFile, next);
   });
 
+  it('should not remove an ::import without the required attributes', done => {
+    expect.assertions(2);
+    const selector = 'leafDirective[name=import]';
+    const plugin = addImports();
+    const importNodes = selectAll(selector, importsExtraAST);
+
+    expect(importNodes.length).toBe(3);
+
+    const next = err => {
+      if (err) {
+        done(err);
+      } else {
+        const importNodes = selectAll(selector, importsExtraAST);
+        expect(importNodes.length).toBe(1);
+        done();
+      }
+    };
+    plugin(importsExtraAST, correctFile, next);
+  });
+
   it('should remove all matching ::use statements', done => {
     expect.assertions(2);
     const selector = 'leafDirective[name=use]';
@@ -205,9 +231,12 @@ describe('replace-imports', () => {
   });
 
   it('should reject imported files with editable region markers', done => {
+    expect.assertions(1);
+    console.error = jest.fn();
     const plugin = addImports();
     const next = err => {
       if (err) {
+        expect(console.error).toHaveBeenCalledTimes(2);
         done();
       } else {
         done('An error should have been thrown by addImports');

tools/challenge-parser/parser/plugins/restore-directives.js

@@ -0,0 +1,32 @@
+const visit = require('unist-util-visit');
+const { matches } = require('unist-util-select');
+const directive = require('mdast-util-directive');
+var toMarkdown = require('mdast-util-to-markdown');
+
+function plugin() {
+  return transformer;
+
+  function transformer(tree) {
+    visit(tree, visitor);
+
+    function visitor(node, id, parent) {
+      // currently `remark-directive` seems to be ignoring containerDirectives
+      // but, assuming that will get fixed, we test for it anyway.
+      const isDirective =
+        matches('leafDirective', node) ||
+        matches('textDirective', node) ||
+        matches('containerDirective', node);
+
+      if (isDirective) {
+        parent.children[id] = {
+          type: 'text',
+          value: toMarkdown(node, {
+            extensions: [directive.toMarkdown]
+          }).trim()
+        };
+      }
+    }
+  }
+}
+
+module.exports = plugin;

tools/challenge-parser/parser/plugins/restore-directives.test.js

@@ -0,0 +1,66 @@
+/* global describe it expect */
+const cloneDeep = require('lodash/cloneDeep');
+const { selectAll } = require('unist-util-select');
+const find = require('unist-util-find');
+
+const restoreDirectives = require('./restore-directives');
+const directivesOriginalAST = require('../__fixtures__/ast-directives.json');
+
+describe('restore-directives', () => {
+  let directivesAST;
+  beforeEach(() => {
+    directivesAST = cloneDeep(directivesOriginalAST);
+  });
+
+  it('should return a function', () => {
+    expect.assertions(1);
+    const plugin = restoreDirectives();
+
+    expect(typeof plugin).toEqual('function');
+  });
+  // TODO: if remark-directive starts processing containers, add them to the
+  // tests
+  it('should remove any directives in the AST', () => {
+    expect.assertions(4);
+    const plugin = restoreDirectives();
+    let leaves = selectAll('leafDirective', directivesAST);
+    let text = selectAll('textDirective', directivesAST);
+    expect(leaves.length).toBe(2);
+    expect(text.length).toBe(2);
+    plugin(directivesAST);
+    leaves = selectAll('leafDirective', directivesAST);
+    text = selectAll('textDirective', directivesAST);
+    expect(leaves.length).toBe(0);
+    expect(text.length).toBe(0);
+  });
+
+  it('should put the original text into the AST', () => {
+    expect.assertions(4);
+    const plugin = restoreDirectives();
+
+    let nodeWithImport = find(
+      directivesAST,
+      node => node.value && node.value.includes('::import')
+    );
+    let nodeWithRoot = find(
+      directivesAST,
+      node => node.value && node.value.includes(':root')
+    );
+
+    expect(nodeWithImport).not.toBeTruthy();
+    expect(nodeWithRoot).not.toBeTruthy();
+    plugin(directivesAST);
+
+    nodeWithImport = find(
+      directivesAST,
+      node => node.value && node.value.includes('::import')
+    );
+    nodeWithRoot = find(
+      directivesAST,
+      node => node.value && node.value.includes(':root')
+    );
+
+    expect(nodeWithImport).toBeTruthy();
+    expect(nodeWithRoot).toBeTruthy();
+  });
+});