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

Setting options interferes with other "require"-calls to marked. #907

Closed
nknapp opened this issue Jun 17, 2017 · 12 comments · Fixed by #2831
Closed

Setting options interferes with other "require"-calls to marked. #907

nknapp opened this issue Jun 17, 2017 · 12 comments · Fixed by #2831
Labels
proposal RR - refactor & re-engineer Results in an improvement to developers using Marked, or end-users, or both.

Comments

@nknapp
Copy link

nknapp commented Jun 17, 2017

In my NodeJS-project, marked is used by the main program and by a dependency.
When the main-program calls setOptions, the same options are also applied to the instance that is used by the dependency, because both instances are the same object.

I think, a better way to apply default options, would be like the request-package is doing it.
They create a new instance that is configured with the default options, so the original instance is untouched.

What I will try now, is to clear the cache before and after requiring marked, but that seems like a weak workaround.

nknapp added a commit to nknapp/thought-plugin-jsdoc that referenced this issue Jun 18, 2017
- Errors during jsdoc-parsing where hardy recognizable as such
- This change makes it clear that they have occured in with jsdoc2md

Related to markedjs/marked#907. It took some time to determine the cause of
an error, because the error message and the stack-trace where not clear.
@Feder1co5oave
Copy link
Contributor

idk if this is viable for you, but you could avoid setting options with setOptions and instead pass options to marked as second argument.

I do agree with you that setOptions should return a new marked instance instead! But that is an API change.

@joshbruce joshbruce added this to the 0.6.0 - Improve developer experience milestone Dec 27, 2017
@joshbruce
Copy link
Member

Again, I'm not too familiar with working directly with Marked; so, pardon the ignorance. Also, this sounds like the problem with Singletons (especially if they're configurable).

var marked1 = Marked().setOptions();
var marked2 = Marked().setOptions();

Is that not the way things work? Sounds like each package would want to create the instance, at which point you would access via something like:

var package = Package();

package.marked.setOptions();

In either case I'm not sure how much this is on Marked to correct. Will flag it for the 0.6.0 milestone; closing.

@Feder1co5oave Feder1co5oave reopened this Feb 23, 2018
@Feder1co5oave Feder1co5oave modified the milestones: 0.6.0 - Improve developer experience, 0.5.0 - Architecture and extensibility Feb 23, 2018
@joshbruce joshbruce modified the milestones: 0.5.0 - Architecture and extensibility, v2.x The great re-engineering Apr 4, 2018
@UziTech UziTech added the RR - refactor & re-engineer Results in an improvement to developers using Marked, or end-users, or both. label Dec 5, 2018
@UziTech
Copy link
Member

UziTech commented Dec 4, 2019

A work around is to use

const marked = require('marked');
marked.setOptions(marked.getDefaults());

to make sure that defaults are set to the initial defaults.

@Andarist
Copy link
Contributor

Andarist commented Dec 4, 2019

The proposed workaround can break those other marked "instances" which is also not good overall.

@UziTech
Copy link
Member

UziTech commented Dec 5, 2019

True. You could also send options as a second parameter to marked to override the defaults just for that call.

const marked = require('marked');
marked(markdown, marked.getDefaults());

@john-doherty
Copy link

I ran into this. As @Feder1co5oave said, the solution is to avoid using marked.setOptions({ renderer: renderer }) and instead pass options inline marked(md, { renderer: renderer }) - worked for me.

@DLiblik
Copy link

DLiblik commented Oct 16, 2022

I hate to rain on a parade, but this is a really clumsy issue. We did not realize this was happening (singleton instance in the background) and to make matters worse, our test pipeline turns over our server as it runs test cases. We had 3 extensions being added when the module was imported (which occurs on each test-reset) and found we had 1,926 extensions installed because marked has no way to reset existing options and it runs as a singleton.

Long gone are the days of polluting the global namespace with singletons like this. If changing the API is a problem, then maybe provide a method to obtain a separate instance with a new method on the API, for ex:

// Get a local instance...
const localMarked = marked.create({/* options here */);

// get some HTML...
const myHtml = localMarked.parse(myMd);

@DLiblik
Copy link

DLiblik commented Oct 16, 2022

...if it's helpful to others, you can clear block extensions (and other defaults - just change the property you are editing) by doing the following, but:

  • this accesses private code 💣
  • you will muck with any other marked instances 💥
if ((marked.defaults as any).extensions?.block?.length) {
  (marked.defaults as any).extensions.block = [];
}

...or, reset the defaults with:

const markedDefaults = {
  async: false,
  baseUrl: null,
  breaks: false,
  extensions: null,
  gfm: true,
  headerIds: true,
  headerPrefix: "",
  highlight: null,
  langPrefix: "language-",
  mangle: true,
  pedantic: false,
  renderer: null,
  sanitize: false,
  sanitizer: null,
  silent: false,
  smartLists: false,
  smartypants: false,
  tokenizer: null,
  walkTokens: null,
  xhtml: false,
};

(marked as any).defaults = {...markedDefaults};

@UziTech
Copy link
Member

UziTech commented Oct 17, 2022

@DLiblik marked.getDefaults() will always return the default options object. You can reset marked with marked.setOptions(marked.getDefaults()).

This is something we have tried changing before with people complaining. It would be great if we can find a way to create an instance without breaking anyone who relys on the singleton.

@rfgamaral
Copy link

This is something we have tried changing before with people complaining. It would be great if we can find a way to create an instance without breaking anyone who relys on the singleton.

Wouldn't a create() method as suggested by @DLiblik above work?

@UziTech
Copy link
Member

UziTech commented Apr 18, 2023

@rfgamaral sure. If you want to create a PR and add tests to make sure it is working we could add a create method

@icebaker
Copy link

fyi we have another issue related to this where I proposed some approaches and also shared a simple workaround that works for my use case. It may be helpful for those struggling in some way, or for those considering making a contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal RR - refactor & re-engineer Results in an improvement to developers using Marked, or end-users, or both.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants