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

IE 11 support :) #1585

Closed
frontendphil opened this issue Dec 17, 2019 · 28 comments
Closed

IE 11 support :) #1585

frontendphil opened this issue Dec 17, 2019 · 28 comments
Labels
category: docs Documentation changes

Comments

@frontendphil
Copy link

I totally would understand if you have no intention of fixing this :) But maybe you could add a note to the README ;) We have just found out that the latest change to you rollup configuration (loose mode) produces output that isn't compatible with IE 11. In particular, it is the object shorthand that IE does not understand.

Describe the bug
The latest marked version does not work in IE 11

To Reproduce
Import it in any file and open this one in IE 11

Expected behavior
It works.

@UziTech
Copy link
Member

UziTech commented Dec 17, 2019

It seems to work just fine for me in IE 11

I created this little test and opened it in ie11 and it seems to work just fine.

<!DOCTYPE html>
<html>
<head>
<title>test</title>
</head>
<body>
	<textarea id="markdown"></textarea>
	<br />
	<button id="button" type="button">Submit</button>
	<br />
	<div id="html"></div>
	<script src="https://cdn.jsdelivr.net/gh/markedjs/marked/lib/marked.js"></script>
	<script>
		var button = document.getElementById("button");
		var html = document.getElementById("html");
		var markdown = document.getElementById("markdown");
		button.addEventListener("click", function () {
			html.innerHTML = marked(markdown.value);
		});
	</script>
	</body>
</html>

Also our demo works fine in ie11 https://marked.js.org/demo/

Can you give us an error message?

@frontendphil
Copy link
Author

Thanks. I think the issue is within the esm build: https://cdn.jsdelivr.net/gh/markedjs/marked@0.8.0/lib/marked.esm.js Look for instance at how the getDefaults method is exported. It uses the object shorthand which it doesn't in the regular build. I guess since we're using webpack it will default to the esm version and not process it further.

@UziTech
Copy link
Member

UziTech commented Dec 17, 2019

Are you saying webpack is not able to process marked.esm.js? or once it is processed it isn't able to work in ie11?

Webpack should be pulling from /src/marked.js. I think you will need to use a babel plugin with webpack to transpile the code to work in whatever browsers you need to support.

@frontendphil
Copy link
Author

Are you saying webpack is not able to process marked.esm.js? or once it is processed it isn't able to work in ie11?

Nope :) What I'm saying is that we have configured webpack to only use babel for everything that is not in node_modules. Not sure why but I definitively see the esm output in our bundle. Generally, this is fine because we want webpack to use the esm bundles so that tree shaking can happen. The modules are then transpiled in a later step. However what is not changed is the object shorthand structure.

Maybe this is my question. Why is the object shorthand transpiled in the marked.js file but not in the esm file? From my understanding, the difference between the two files should only be that the one features es module syntax and the other doesn't.

@UziTech
Copy link
Member

UziTech commented Dec 17, 2019

ie11 doesn't support esm so there should be no reason to transpile it to work with ie11.

Most likely webpack is pulling code from /src/marked.js and bundling it but not transpiling it so it would look a lot like /lib/marked.esm.js without the exports

The modules are then transpiled in a later step. However what is not changed is the object shorthand structure.

Seems like your transpiler should change it to work with your supported browsers in this transpile step.

@oculus42
Copy link

The core issue is a breaking change was made to the package without a server major release, so consumers who have successfully used the code in the past are suddenly impacted.

Would you consider publishing a 0.8.1 (or 0.9.0) which reinstates the previous release and re-publishing 0.8.0 as 1.0.0?

@UziTech
Copy link
Member

UziTech commented Dec 17, 2019

@oculus42 We do follow semver for 0.x.y where x is breaking changes and y is non-breaking changes.

@smhg
Copy link
Contributor

smhg commented Dec 18, 2019

Practically, if you're using browserify+babel(ify) (and possibly other tools):

Where with 0.7 you'd use:

const md = require('marked');
// or
import md from 'marked';

If you need IE 11 support, with 0.8 this now needs to be:

const md = require('marked/lib/marked');
// or
import md from 'marked/lib/marked';

@frontendphil
Copy link
Author

Ok, now I also see that the main part of the package.json actually points to ./src/marked.js. That's probably the confusion. However, I'm wondering why this is the case. Why the rollup config and build steps if what your consumers get is the raw source after all?

@smhg
Copy link
Contributor

smhg commented Dec 18, 2019

@UziTech @frontendphil a solution could be to have package.json's main point to lib/marked.js and add a module entry which points to src/marked.js (webpack, rollup,... use that if available). This might also make the ESM build obsolete?

That is, if this needs a solution. I'm not very up to date with the current state of transpiling.

@frontendphil
Copy link
Author

We usually have this in our rollup configs:

{
  output: [
    {
      file: 'lib/marked.cjs.js',
      format: 'cjs',
      compact: true,
    },
    {
      file: 'lib/marked.esm.js',
      format: 'esm',
      compact: true,
    },
  ],
}

You can let your 'main' point to lib/marked.cjs.js and module to lib/marked.esm.js. Both should still have everything else transpiled through the standard babel config you already have in your rollup config. Directly pointing at your src folder isn't a good practice IMHO if you want to use some more modern language features while developing but ship something that works nonetheless.

@UziTech
Copy link
Member

UziTech commented Dec 18, 2019

Why the rollup config and build steps if what your consumers get is the raw source after all?

Because marked can be a client side library without node by using

<script src="https://cdn.jsdelivr.net/gh/markedjs/marked/lib/marked.js"></script>

or

import marked from 'https://cdn.jsdelivr.net/gh/markedjs/marked/lib/marked.esm.js';

You can let your main point to lib/marked.cjs.js and module to lib/marked.esm.js

see discussion #1571 (comment)

Also benchmark testing showed using es6 in /src/marked.js in node was about 10% faster than using the babel generated es5 code in /lib/marked.js.

@frontendphil
Copy link
Author

Alright, I would just not assume that I need to explicitly import from the lib folder in order to get this to work. At least I would think that this should be mentioned in the readme.

@UziTech UziTech added category: docs Documentation changes and removed need more info labels Dec 18, 2019
@smhg
Copy link
Contributor

smhg commented Dec 18, 2019

That's only one pull request away! 🙂

@oliviertassinari
Copy link

I have observed the same behavior, https://material-ui.com/ crashes in IE 11 since the upgrade of marked from v0.7.0 to v0.8.0 mui/material-ui#18834.

Capture d’écran 2019-12-26 à 22 34 01

@oliviertassinari
Copy link

oliviertassinari commented Dec 26, 2019

Given this comment:

Because marked can be a client side library without node by using
<script src="https://cdn.jsdelivr.net/gh/markedjs/marked/lib/marked.js"></script>

It seems that the build is configured as follow:

  • /src: the source, raw. They shouldn't be used unless a developer wants to transpile the source to target a specific set of browsers.
  • /lib/marked.js: the UMD module that targets older browsers, perfect for CDN delivery.
  • /lib/marked.esm.js: the UMD module that targets browsers that support es modules, perfect for CDN delivery.

I would propose to add a CJS version of the package and to link it:

package.json

{
  "name": "marked",
  "description": "A markdown parser built for speed",
  "author": "Christopher Jeffrey",
  "version": "0.8.0",
- "main": "./src/marked.js",
+ "main": "./lib/marked.cjs.js",
  "bin": "./bin/marked",
  "man": "./man/marked.1",

oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Dec 29, 2019
oliviertassinari added a commit to oliviertassinari/material-ui that referenced this issue Dec 29, 2019
@xygon
Copy link

xygon commented Jan 4, 2020

I have the same issue, upgrading from 0.7.0 to 0.8.0 causes a failure for my angular 8 site in IE11
SCRIPT1002: Syntax error main.f94e31e3fb9286b4b630.js (1,89)
which resolves to
const { defaults } = __webpack_require__(/*! ./defaults.js */ "./node_modules/marked/src/defaults.js");
This is an issue as IE11 doesn't support destructuring assignment.

@UziTech
Copy link
Member

UziTech commented Jan 5, 2020

@xygon the work around is to use require('marked/lib/marked.js') or transpile marked with babel.

@deshiknaves
Copy link

@oculus42 We do follow semver for 0.x.y where x is breaking changes and y is non-breaking changes.

I'm just going to leave this here: semver version 2

and npx check-update --update

image

@styfle
Copy link
Member

styfle commented Jan 9, 2020

Your linked specification for Semver 2.0.0 says the following in item 4:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

Therefore it is following the semver spec.

The plan is to release 1.0.0 once the API is stable and commonmark + gfm tests pass.

@deshiknaves
Copy link

Yep. That's fair. We'll get another library — this is obviously not stable enough for a normal workflow. Cheers.

@UziTech
Copy link
Member

UziTech commented Jan 9, 2020

@deshiknaves I would recommend markdown-it They are very fast, extensible, and have quite a few plugins for GFM and other variations.

@wmarques
Copy link

This workaround should be documented in the README IMO, do you want me to provide a PR for this ?

@UziTech
Copy link
Member

UziTech commented Feb 19, 2020

@wmarques yes please

@joshbruce
Copy link
Member

Good conversation here. Just to throw a bit on the fire: #1522

Anyone is welcome to help move Marked along toward the 1.0 future. Will probably post an update to the project status concept in the coming months. Sticking with the theme it'll probably be called: If only we had a holocaust cloak. :)

@tomwheyprotein
Copy link

tomwheyprotein commented Mar 7, 2020

I recently ran into a version mismatch between marked@0.8.0 and @types/marked@0.7.3. After downgrading marked to 0.7.0, our builds worked with IE.

There is no package version for @types/marked@0.8.0 available at this time.

@jebarjonet
Copy link

Seriously, exporting /src source by default instead of the dist version... 🙈

j4kim added a commit to cret-vaillant/camping-vaillant that referenced this issue Mar 10, 2020
Ionaru added a commit to Ionaru/easy-markdown-editor that referenced this issue Mar 20, 2020
bugfix for IE compatibility, thanks to markedjs/marked#1585 (comment)
@UziTech
Copy link
Member

UziTech commented May 21, 2020

This should be fixed by #1661

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

No branches or pull requests