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

0.29.2 breaks webpack build #195

Closed
m-mohr opened this issue Sep 23, 2020 · 13 comments
Closed

0.29.2 breaks webpack build #195

m-mohr opened this issue Sep 23, 2020 · 13 comments

Comments

@m-mohr
Copy link

m-mohr commented Sep 23, 2020

Since 0.29.2 I'm getting the following error when building a Vue component that uses commonmark.js:

"export 'default' (imported as 'commonmark') was not found in 'commonmark'

Im importing with import commonmark from 'commonmark';.

I'm building with vue-cli and that uses webpack again. I suspect that #186 broke it, at least I can make it work by downgrading to 0.29.1.

@jgm
Copy link
Member

jgm commented Sep 23, 2020

See the discussion in #136.

I'm sorry about this; I don't use JavaScript myself and I'm not up on the module technology. As far as I could tell, #136 was doing the right thing, but maybe not. It may work to do import * as commonmark from 'commonmark', as suggested in the discussion there. But it may be that we are doing things in a non-standard way that violates expectations (and creates problems with backwards-compatibility). IN that case, I'd like to fix it, but I'll need help from people who know what they're doing with JS modules.

@m-mohr
Copy link
Author

m-mohr commented Sep 24, 2020

Thank you, I'm also not really into all the details about JS modules.

I guess it would help to add to the readme how to include the library. I guess most people would try import commonmark from 'commonmark' instead of import * as commonmark from 'commonmark'. For me it seems that the default export is missing and should be added. Anyway, the proposed solution works for now, but as it's a breaking change (I think), it might be a good idea to fix things in 0.29.3.

@jgm
Copy link
Member

jgm commented Sep 24, 2020

Sure. How do I fix it so that import commonmark from 'commonmark' works again?
And, will that break import * as commonmark from 'commonmark'?

@m-mohr
Copy link
Author

m-mohr commented Sep 24, 2020

Again, I'm not an expert and haven't checked it, but I'd assume something like that should work in index.js:

import Node from "./node.js"
import Parser from "./blocks.js";
import Renderer from "./render/renderer.js";
import HtmlRenderer from "./render/html.js";
import XmlRenderer from "./render/xml.js";

const lib = { Node, Parser, Renderer, HtmlRenderer, XmlRenderer };
export lib; // makes `import * as commonmark from 'commonmark'` work
export default lib; // makes `import commonmark from 'commonmark'` work

But better let other experts double check that this works.

@hp4k1h5
Copy link

hp4k1h5 commented Sep 24, 2020

not an expert but this is my understanding as well, at least for node 14.11+

export lib; // makes import * as commonmark from 'commonmark' work
export default lib;

this export setup should provide the two import styles, as well as

import { lib } from './lib.js' // same as import * as lib from './lib.js'

@jgm
Copy link
Member

jgm commented Sep 25, 2020

So, is this the normal thing to do, making both styles of import work?

@m-mohr
Copy link
Author

m-mohr commented Sep 25, 2020

No, usually I'd only expect the export default thing that allows me to do import commonmark from 'commonmark'. I've not seen many libraries do import * as commonmark from 'commonmark'. Anyway, 0.29.2 introduced it and some people adopted, so to not break things again it seems it must support both from now on?!

@jgm jgm closed this as completed in 09dc357 Sep 25, 2020
@jgm
Copy link
Member

jgm commented Sep 25, 2020

For some reason export lib didn't work. I had to use a constant there instead.
But I've adopted this suggestion.

I'm not sure about whether to continue supporting import * as commonmark, but we should probably at least support it in the next release, with a deprecation notice.

@jgm
Copy link
Member

jgm commented Sep 25, 2020

OK, having both styles seems to be a problem:
https://github.com/commonmark/commonmark.js/runs/1166893414?check_suite_focus=true
I think I'll just go to the preferred style and people will have to deal with the breakage.
Fortunately 0.29.2 was very recently released, so I hope it won't affect too many people.

@jgm jgm reopened this Sep 25, 2020
@jgm
Copy link
Member

jgm commented Sep 25, 2020

Actually, I'm going to reopen this and try to get more feedback.
I see that some libraries use one style (e.g. entities), while others use the other.
I'm not sure what is best, but supporting both seems to be impossible.
Advice welcome from people who know more about this than I do!

@jgm
Copy link
Member

jgm commented Sep 25, 2020

I've reverted the changes related to this issue, for now.

nikolas added a commit to ccnmtl/econplayground.js that referenced this issue Oct 19, 2020
This required some manual changes, since these scripts have been adapted
from an old ejected create-react-app config in this project.

Requires a commonmark.js downgrade:
commonmark/commonmark.js#195
nikolas added a commit to ccnmtl/econplayground.js that referenced this issue Oct 19, 2020
This required some manual changes, since these scripts have been adapted
from an old ejected create-react-app config in this project.

Requires a commonmark.js downgrade:
commonmark/commonmark.js#195
nikolas added a commit to ccnmtl/econplayground.js that referenced this issue Oct 19, 2020
This required some manual changes, since these scripts have been adapted
from an old ejected create-react-app config in this project.

Requires a commonmark.js downgrade:
commonmark/commonmark.js#195
@jgm
Copy link
Member

jgm commented Dec 4, 2020

This should be fixed by #203

@jgm jgm closed this as completed Dec 4, 2020
@MartinX3
Copy link

MartinX3 commented Dec 23, 2020

I'm still getting "export 'default' (imported as 'commonmark') was not found in 'commonmark' with 0.29.3 in vueJS 2.x
Or I understand the PR wrong

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

No branches or pull requests

4 participants