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

Merge from khan/simple-markdown #2

Merged
merged 48 commits into from
Jan 2, 2020
Merged

Conversation

andangrd
Copy link
Owner

@andangrd andangrd commented Jan 2, 2020

No description provided.

ariabuckles and others added 30 commits August 4, 2019 18:13
Addresses #71

Inline code has an exponential backtracking regex.
This commit makes tests for those so that we can verify we fix them.

Test plan:

1. Run `make test`
    * Verify the three exponential backtracking avoidance tests fail
Our inline code regex had overlapping parts in the case of spaces;
spaces could be parsed as part of the `\s*`s or as part of the
`[\S\s]*`, which leads to catastrophic backtracking in the case of
a string with many spaces.

The `\s*` parts were attempting to allow for escaping of backticks
within the start/end of inline code blocks, so this commit removes
the `\s*` parts of the regex, and then after parsing the code block,
checks for the case where a single space is being used to escape
a backtick, and removes that space.

Test plan:

1. Added a test for the semantics change.
2. Added a test for the exponential backtracking to match the test
   case in issue #71
Test plan:

1. `make test`
    * Verify all tests now pass
$1 only refers to the parens in the first branch of the `` /^ ( *` *) $|^ ( *`)|(` *) $/g `` disjunction. If another branch matches, the replacement will be blank.
This adds an additional `globalPrevCapture` that is "global" state for the parses and persist into nested invocations of `nestedParse`. This allows rules to determine if they are truly at the beginning of a line. The existing `prevCapture` resets at each nesting, so it only allows you to determine if you're at the beginning of the nested parse, but not necessarily the line.
Minor change; most of the other comparisons work this way, and this
makes prevCapture harder to break
Fix #72: backticks in code blocks bug
Add #66: state.prevCapture that persists between parse calls
#70: Bump devDependency versions for security
Patterns like <<<<<<<<<<:/:/:/:/:/:/:/:/:/:/ currently exhibit O(n^3) complexity, allowing a 5KB document to take 7174ms to parse. With this change, it drops to O(n^2) and 73ms.
ES6 import usage is:

```javascript
import * as SimpleMarkdown from 'simple-markdown';
```

so rather than export a default object, we should be exporting everything.
Just a small fixup of some flow types so that when adding the typescript
types they're more of a straight port.

Done before the typescript changes so this change is easier to verify on
its own.

Test plan: `make check`
This cleanup happens in two parts:

1. We remove some logic to disable certain error handling, which, as
   far as I can tell, was never actually used :/. Doing this makes
   both typescript and flow happier (and it's harder to work around
   in typescript than it was in flow).

    * In order to avoid introducing a potential perf regression for
      anyone who could have been using `state.disableErrorGuards`,
      we also remove the more expensive string comparison check, and
      instead only check for missing `^` if the result is a raw
      regex capture result (i.e., if it has a numeric index, we warn
      if that index is non-zero).

2. We add an error check if the Array joiner rule isn't defined for
   a non-html/react output type.

    * This will make it easier to convince typescript that the function
      we call isn't undefined, and is a better api/documentation as
      well.

Test plan:

As... noted in the comment, these parts aren't really tested in the
tests, but I did run the tests to verify that the normal path API
didn't seem to break.
ariabuckles and others added 18 commits October 24, 2019 16:21
Updates type type definition file to be equivalent to the flow typings.

Test plan:

I didn't test this externally; will ask someone using typescript to check
whether this is working for their usecase.

The contents of the file are tested later with `make check` though.
Add typescript types to our source, so that we can use typescript in
addition to flow to check our source, and verify that our types are
correct.

Test plan: tested in later commit that enables tsc in `make check`
Install typescript and configure it to check simple-markdown.js &
simple-markdown.d.ts during `make check`

Test plan: `make check`
Adds types to simple-markdown-test.js and checks that file with
typescript as well \o/

Test plan: `make test`
Adds some integration for rollup/the rollup changes with some other
parts of the build system:

 * `make` targets
 * fixes typescript config
 * uses es6 module exports to switch to fully using rollup's umd
 * integrates rollup with npm prepublish so I can't forget to build
As far as I can tell, this wasn't being used, and was creating an unused
variable.

Test plan: `make build test`
Test plan: `npm ci && make test`
Test plan: `make test`
Now that we're bundling with rollup, we shouldn't need the @types/node
dependency, which can make installation more challenging.

Fixes #80
@andangrd andangrd merged commit 48ebe9e into andangrd:master Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants