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

fix: export cjs/esm/umd modules explicitly #2747

Closed
wants to merge 1 commit into from
Closed

fix: export cjs/esm/umd modules explicitly #2747

wants to merge 1 commit into from

Conversation

laustdeleuran
Copy link

Marked version: 4.2.12

Markdown flavor: n/a

Description

Expectation

Including marked in a subpackage should work as expected.

Result

I wish I had a more reproducible setup.

In short, we have a monorepo using pnpm. I have a package in that repo that uses marked. The file in question looks something like:

import DOMPurify from 'isomorphic-dompurify';
import { marked } from 'marked';

marked.use({
  renderer: {
    /* ...some options here */
  },
});

export const format = (markdown: string) =>
  DOMPurify.sanitize(marked.parse(markdown, { breaks: true }));

That package compiles that to CJS, which ends up looking something like:

"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});
exports.format = void 0;

var _isomorphicDompurify = _interopRequireDefault(require("isomorphic-dompurify"));

var _marked = require("marked");

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

_marked.marked.use({
  renderer: {
    /* ...some options here */
  }
});

const format = markdown => _isomorphicDompurify.default.sanitize(_marked.marked.parse(markdown, {
  breaks: true
}));

exports.format = format;

That module then gets imported in another project in the monorepo, which is a create-react-app. I'm trying to upgrade the react-scripts in that project, which means I'm also upgrading from Webpack 4 to Webpack 5. That upgrade is going smoothly, except for the above file now throwing the TypeError: Cannot read properties of undefined (reading 'use') error.

What was attempted

If I explicitly export the three generated files, which should be safe to mark as exports, I'm able to sidestep this issue in my downstream projects using resolve:

    "alias": {
      "marked": "marked/lib/marked.esm.js",
    },

Which in effect applies @emilmuller's wrokaround to Webpack 5.

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
  • no tests required for this PR.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

@vercel
Copy link

vercel bot commented Mar 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
marked-website ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 2, 2023 at 11:36PM (UTC)

Comment on lines +26 to +28
"./lib/marked.cjs": "./lib/marked.cjs",
"./lib/marked.esm": "./lib/marked.esm.js",
"./lib/marked.umd": "./lib/marked.umd.js",
Copy link
Author

Choose a reason for hiding this comment

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

question (non-blocking): I'm a little unsure on the naming here. Should the exports include file types?

Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

this seems like the wrong fix. I think you should figure out the root of your issue rather than introducing a workaround into marked

@laustdeleuran
Copy link
Author

this seems like the wrong fix. I think you should figure out the root of your issue rather than introducing a workaround into marked

That's a fair critique. I was mainly just trying to illustrate more clearly what the workaround was.

But, I don't think this really falls into introducing a workaround into marked itself. I'm just exposing files that are already available to consumers.

But your point still stands.

@laustdeleuran laustdeleuran closed this by deleting the head repository Mar 10, 2023
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.

module import/export issue
2 participants