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

Modernizing the codebase #1539

Closed
Andarist opened this issue Aug 22, 2019 · 6 comments · Fixed by #1563
Closed

Modernizing the codebase #1539

Andarist opened this issue Aug 22, 2019 · 6 comments · Fixed by #1563

Comments

@Andarist
Copy link
Contributor

I'm wondering if you'd be interested in modernizing the codebase and introducing a build step.

Why this could be beneficial? Currently marked in written solely in UMD format and this makes modern bundlers unable to even attempt at tree-shaking it. For example, I'd like to only use Lexer class and render things myself, but currently I end up with everything else what's exported by marked.

I, of course, offer my help to implement this - I would only make sure first if that's something you'd like to see happen.

Note I don't plan to introduce any breaking changes, the output files should still work in all currently supported environments etc.

@joshbruce
Copy link
Member

Hello @Andarist.

We are currently working on ensuring compliance with currently supported standards; see the milestones - 0.x specifically.

The 1.x and 2.x releases would be when something like this makes the most sense, I think.

If this is something you're passionate about, please do submit a PR on what that might look like in your mind. With that said, it may need to take second seat to security and markdown standard fixes.

@UziTech
Copy link
Member

UziTech commented Aug 22, 2019

There are a couple PRs to update marked with a build step. #1349 #1452

The problem with these PRs and updating marked to with a build step is that marked was originally created to be used in the browser and I believe we still have quite a few dependents using jsdelivr to include marked on the client side.

We would need a build step to create /lib/marked.js and /marked.min.js as a single file that would work with older browsers.

@Martii
Copy link
Contributor

Martii commented Aug 22, 2019

... I believe we still have quite a few dependents using jsdelivr to include marked on the client side.

If I'm reading and applying this right, I brought up on another package (and they made an issue for it) we would prefer to have a static default build served alternately on GH in case npmjs.com is down depending on where we are at in development vs. production. Unforseen events can always happen with hosting. If npmjs.com were down we'd have to incorporate a build step in a hurry unless it was fully transparent with our static linking. We've already taken some precautions against npmjs.com outages but in a pickle there could be some events that could potentially take us down if our deps aren't available or if the main hosting maintainer is on break (that would be me usually).

@UziTech
Copy link
Member

UziTech commented Aug 22, 2019

Ideally we could offer an ES module output for anyone using a bundler on node and output /lib/marked.js and marked.min.js with a build step as files on GitHub for anyone using a static build.

We currently have a build step to minify marked automatically on every successful push to master
https://github.com/markedjs/marked/blob/master/.travis.yml#L23

@Martii
Copy link
Contributor

Martii commented Aug 23, 2019

... a build step to minify marked automatically ...

Cool beans. Would be helpful to maintain the unminified too when this point comes for this issue (and the others) on the back-(seat)-burner. We currently use express-minify to minify cache everything we serve, including marked here, which in turn calls terser to do the actual one time minification so we don't end up doing it twice CPU wise. Obviously we can restrict express-minify from the marked path but prefer to keep all sources in the unadulterated state for debugging if needed. That other package I referenced is going to make it more time consuming to debug since we'll possibly get future errors in JavaScript but then it will need to be tracked down in TypeScript... kind of a bummer of the process with "modernizing" making issue reporting more complex.

@UziTech
Copy link
Member

UziTech commented Aug 23, 2019

kind of a bummer of the process with "modernizing" making issue reporting more complex.

I agree, we will try to keep /lib/marked.js unminified for easier debugging on older browsers.

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

Successfully merging a pull request may close this issue.

4 participants