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

feat: merge beta #947

Merged
merged 236 commits into from
Aug 8, 2024
Merged

feat: merge beta #947

merged 236 commits into from
Aug 8, 2024

Conversation

kellyjosephprice
Copy link
Collaborator

@kellyjosephprice kellyjosephprice commented Aug 5, 2024

PR App

🧰 Changes

Merges beta into next!

🧬 QA & Testing

BREAKING CHANGE: This is a near complete rewrite of the library, and converts it to supporting MDX instead of markdown.

kellyjosephprice and others added 11 commits July 11, 2024 20:48
| [![PR App][icn]][demo] | RM-9793 |
| :--------------------: | :-----: |

## 🧰 Changes

Changes how tables are saved as JSX.

The align attribute is stored on the `Table` component, as well as the
table cells:

```
<Table align={["center", "center"]}>
  <thead>
    <tr>
      <th style={{ textAlign: 'center' }}>Hi!</th>
      <th style={{ textAlign: 'center' }}>Hello!</th>
    </tr>
  </thead>
  <tbody>
    <tr>
      <td style={{ textAlign: 'center' }}>Bye!</td>
      <td style={{ textAlign: 'center' }}>Bye Bye!</td>
    </tr>
  </tbody>
</Table>
```

It also includes some cleanup of code that's not being used, namely
`html`, `esast`, and `hastFromHtml`. `hastFromHtml` was going to be used
for search indexing, but I think we're going to switch to much simpler
parsing of just the `hast`.

## 🧬 QA & Testing

- [Broken on production][prod].
- [Working in this PR app][demo].

[demo]: https://markdown-pr-PR_NUMBER.herokuapp.com
[prod]: https://SUBDOMAIN.readme.io
[icn]:
https://user-images.githubusercontent.com/886627/160426047-1bee9488-305a-4145-bb2b-09d8b757d38a.svg
## Version 6.75.0-beta.73

### 🛠 Fixes & Updates

* tables ([#938](#938)) ([1cbb175](1cbb175))

<!--SKIP CI-->
## Version 6.75.0-beta.74

### 🛠 Fixes & Updates

* use td ([47650a4](47650a4))

<!--SKIP CI-->
| [![PR App][icn]][demo] | RM-9793 |
| :--------------------: | :-----: |

## 🧰 Changes

Skips the align attrs if they're all null.

I didn't realize markdown tables default to null, rather than a string:

```
| foo |
| --- |
```

## 🧬 QA & Testing

- [Broken on production][prod].
- [Working in this PR app][demo].

[demo]: https://markdown-pr-PR_NUMBER.herokuapp.com
[prod]: https://SUBDOMAIN.readme.io
[icn]:
https://user-images.githubusercontent.com/886627/160426047-1bee9488-305a-4145-bb2b-09d8b757d38a.svg
## Version 6.75.0-beta.75

### 🛠 Fixes & Updates

* skip null align ([#940](#940)) ([7880a89](7880a89))

<!--SKIP CI-->
| [![PR App][icn]][demo] | RM-9793 |
| :--------------------: | :-----: |

## 🧰 Changes

Fixes saving 'complex' tables.

In a prior PR, I added a new node type `tableau` to represent tables
with block content. This mostly worked for the editor, but not when
doing `mdx(mdast(doc))`. This PR adds a transformer, so the a `tableau`
is converted to a JSX table just like regular `table`'s.

This also adds the check so, if a `tableau` no longer has block content,
it's saved as markdown!

## 🧬 QA & Testing

Hard to test outside the main app.

- [Broken on production][prod].
- [Working in this PR app][demo].

[demo]: https://markdown-pr-PR_NUMBER.herokuapp.com
[prod]: https://SUBDOMAIN.readme.io
[icn]:
https://user-images.githubusercontent.com/886627/160426047-1bee9488-305a-4145-bb2b-09d8b757d38a.svg
## Version 6.75.0-beta.76

### 🛠 Fixes & Updates

* saving tables ([#942](#942)) ([50c392e](50c392e))

<!--SKIP CI-->
| [![PR App][icn]][demo] | RM-9793 |
| :--------------------: | :--------: |

## 🧰 Changes

Fixes saving lists (other blocks too) in table headers.

## 🧬 QA & Testing

Kind of hard to test outside the editor. But I've been testing by
running `make mdx` and playing around in the styleguidist playground
thingy.

- [Broken on production][prod].
- [Working in this PR app][demo].

[demo]: https://markdown-pr-PR_NUMBER.herokuapp.com
[prod]: https://SUBDOMAIN.readme.io
[icn]:
https://user-images.githubusercontent.com/886627/160426047-1bee9488-305a-4145-bb2b-09d8b757d38a.svg
## Version 6.75.0-beta.77

### 🛠 Fixes & Updates

* lists in table headers ([#943](#943)) ([6a2e80f](6a2e80f))

<!--SKIP CI-->
import React, { useEffect } from 'react';
import { renderToStaticMarkup } from 'react-dom/server';

const MATCH_SCRIPT_TAGS = /<script\b[^>]*>([\s\S]*?)<\/script *>\n?/gim;

Check failure

Code scanning / CodeQL

Bad HTML filtering regexp High

This regular expression does not match script end tags like </script\\t\\n bar>.
while ((match = MATCH_SCRIPT_TAGS.exec(html)) !== null) {
scripts.push(match[1]);
}
const cleaned = html.replace(MATCH_SCRIPT_TAGS, '');

Check failure

Code scanning / CodeQL

Incomplete multi-character sanitization High

This string may still contain
<script
, which may cause an HTML element injection vulnerability.
@kellyjosephprice kellyjosephprice marked this pull request as ready for review August 6, 2024 21:45
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Comment on lines 71 to +76
"@babel/plugin-proposal-class-properties": "^7.18.6",
"@babel/plugin-proposal-object-rest-spread": "^7.19.4",
"@babel/plugin-proposal-optional-chaining": "^7.18.9",
"@babel/plugin-transform-runtime": "^7.24.0",
"@babel/preset-env": "^7.24.0",
"@babel/preset-react": "^7.23.3",
"@commitlint/cli": "^19.0.3",
"@commitlint/config-conventional": "^19.0.3",
"@readme/eslint-config": "^13.5.0",
"@readme/stylelint-config": "^6.1.0",
"@babel/plugin-proposal-export-default-from": "^7.23.3",
"@babel/plugin-proposal-object-rest-spread": "^7.20.7",
"@babel/plugin-proposal-optional-chaining": "^7.21.0",
"@babel/plugin-proposal-private-methods": "^7.18.6",
"@babel/plugin-transform-runtime": "^7.21.4",
Copy link
Member

Choose a reason for hiding this comment

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

doesn't need to happen here but im curious which of these plugins are still needed

@@ -2,113 +2,132 @@
"name": "@readme/markdown",
"description": "ReadMe's React-based Markdown parser",
"author": "Rafe Goldberg <[email protected]>",
"version": "6.87.1",
"version": "6.75.0-beta.77",
Copy link
Member

Choose a reason for hiding this comment

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

not sure how the package publishing stuff works here but should all of this qualify for a major v7 release since i assume it's all a pretty big breaking change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes definitely, forgot to do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, updated the commit body with BREAKING CHANGE:, hopefully that changes the major number 🤞🏼

"@commitlint/config-angular": "^17.4.4",
"@commitlint/config-conventional": "^17.4.4",
"@readme/eslint-config": "^14.0.0",
"@readme/markdown-legacy": "npm:@readme/markdown@^6.84.0",
Copy link
Member

Choose a reason for hiding this comment

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

without doing a v7 release this legacy package import is likely going to start importing the all of this new work

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Once the migration is done, I can probably delete that.

Copy link
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

LGTM

@kellyjosephprice kellyjosephprice merged commit b9502ad into next Aug 8, 2024
12 of 13 checks passed
@kellyjosephprice kellyjosephprice deleted the feat/merge-beta branch August 8, 2024 20:39
rafegoldberg pushed a commit that referenced this pull request Aug 8, 2024
## Version 6.88.0

### ⚠ BREAKING CHANGES

* CHANGE: This is a near complete rewrite of the library, and converts it to supporting MDX instead of markdown.

### ✨ New & Improved

* merge beta ([#947](#947)) ([b9502ad](b9502ad))

<!--SKIP CI-->
@rafegoldberg
Copy link
Contributor

This PR was released!

🚀 Changes included in v6.88.0

@rafegoldberg
Copy link
Contributor

(ooof, squash merging this caused us to loose all the linked PR-to-commit history from the last years worth of work on the beta branch… makes it real hard to track where/why certain changes were introduced.)

@kellyjosephprice
Copy link
Collaborator Author

(ooof, squash merging this caused us to loose all the linked PR-to-commit history from the last years worth of work on the beta branch… makes it real hard to track where/why certain changes were introduced.)

Yea, a lot of the commits were poor quality though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants