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

Declare undeclared variables #991

Merged
merged 2 commits into from Jan 5, 2018
Merged

Conversation

learykara
Copy link
Contributor

@learykara learykara commented Dec 27, 2017

Fix for #990

These two variables are undeclared and throw an error in strict mode.

@joshbruce
Copy link
Member

@learykara: Thank you for the PR. Is there a way to write a test for this?

Want to make sure we don't get a regression.

I don't know enough about the Marked test suite to know and I think it's somewhat custom.

@Feder1co5oave
Copy link
Contributor

Adding "use strict"; at the top of lib/marked.js puts the engine in strict mode and doesn't compile without this fix. I don't know the consequences of doing this, though. Might need to hear someone about this.

@joshbruce
Copy link
Member

Let's tag in @matt- and @UziTech, see if they can weigh in.

@felixfbecker
Copy link

felixfbecker commented Jan 5, 2018

I agree that adding 'use strict' would be best (and if tests pass that should mean everything is alright) but it would be great to get this fix released asap. The last working version 0.3.8 has a security vulnerability that is currently alerted by GitHub with the recommendation to update to 0.3.9, which breaks everyone with a dependency on marked in a strict mode environment at runtime.

@felixfbecker
Copy link

This can also be prevented with eslint (no-unused-variable)

@UziTech
Copy link
Member

UziTech commented Jan 5, 2018

LGTM @learykara could you add "use strict"; at the top of the function (line 8)

@learykara
Copy link
Contributor Author

done @UziTech !

@joshbruce
Copy link
Member

All right. Lots of good conversation here.

Here's the plan:

  1. Merging this.
  2. Prepping a 0.3.10 release - please flag other PRs you think fix defects that should be included in that release within the next two days. So, plan is to have a release on Monday.
  3. Starting a ticket to discuss adding a linter to Marked. See Add eslint and rules to Marked #999

Anything else we need to do?

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.

None yet

5 participants