-
Notifications
You must be signed in to change notification settings - Fork 64
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
"export 'default' (imported as 'require$$0') was not found in 'acorn' #159
Comments
I think this could be a problem with rollup and the way you are configuring |
Actually I think this is the culprit |
Actually I think this is a problem with conflicting acorn versions in the node_module tree. for some reason my |
I have investigated on this issue a bit and here are my findings so far :
And I checked, no other version of acorn is loaded in this bundle.
in
Which is the culprit as we now know acorn doesn't have a default export, and this if we look where it is used we get
Which come respectively from
As you can see, in both cases, the require is done with My proposal to treat all node_modules as externals in the case of a browser build, Webpack can handle those imports fine. I'll make a pull request for my proposed change |
I just don't understand it. If this is a hard dependency why configure it as external? Why this is not a peerDependency then? I may be missing something here, but if you put it in "dependencies" it makes sense to me to bundle it together. |
No, it's the other way round: If you don't configure it as external you shouldn't have it as dependency, since you are not actually |
is this a rollup concept? Sorry, I feel I'm definitely missing something, but, I'm using webpack + dependencies and I never had to configure external in webpack. I let webpack bundle everything together and create separate chunks when my bundle size gets big. |
I guess this is a rollup concept. With rollup, you can specify whether dependencies should be put in the bundle or referenced dynamically. If I don't specify any modules as |
yeah, I think there is something weird with rollup and the way they are converting the require to import as @onigoetz said. This is a "blocker" issue for me, I had to remove the live code editor from https://v2.grommet.io. I will add it back once we figure this out. |
Yes, |
I hear you. In a project that I support I do declare externals for react and react dom https://github.com/grommet/grommet/blob/NEXT/webpack.config.babel.js#L24 But they are peerDependencies in my package.json. https://github.com/grommet/grommet/blob/NEXT/package.json#L41 |
I don't think this issue would be solved by using |
I'm 90% sure this whole issue is from bumping |
That import in |
I don't see a default import there. |
See @onigoetz's comment on how that gets transpiled. |
Ah, now I get it. Does this only happen with webpack? |
I'm not sure, looking in to it. To be clear on the change from 0.19.4, acorn-jsx: ^4.1.1 import MagicString from 'magic-string';
import * as acorn from 'acorn'; 0.19.5, acorn-jsx: ^5.0.0 import require$$0, { Parser } from 'acorn'; |
The default |
@alansouzati Externals and peerDependencies are two completely different concepts. PeerDependencies are handled at npm level, it made sense back to npm 2, when the tree of dependency respected the definition of the packages, Those were a trick to allow multiple packages to leverage one dependency while only one needed to actually depend on it. externals is a concept for bundlers, let me explain it this way, you provide a library with a react component that has prop-types, so you have a dependency on
Option 1 is great when you are creating an application, it's the bundle that you're going to ship in the browser, it's logical to have everything inside. But in the case of a library, when importing it, if your application also requires React and PropTypes, it will have to re-import them in your bundle, and you will end up with code that is duplicated in your bundle without a way to remove it. Option 2 on the other hand, leverages npm's dependencies, because it's able to resolve to the latest version of a dependency and your bundler is able to resolve your dependency and de-duplicate it for you |
Just tested with @onigoetz's PR #161 which fixes this. The resulting imports are: import rewritePattern from 'regexpu-core';
import MagicString from 'magic-string';
import { Parser } from 'acorn';
import acornJsx from 'acorn-jsx';
import acornDynamicImport from 'acorn-dynamic-import'; Which works nicely with webpack. |
I'm ambivalent on whether the other dependencies should be bundled or not, but definitely fully externalizing acorn/acorn-jsx solves the issue. |
Does #162 fix it, too? |
Ok, so far my understanding is that
My current plan to fix this issue:
I'm assuming that my tests from #164 are enough to establish whether our build artifacts are correct or not, so if you have any further bundler setups that are broken, please post them. |
Yeah a quick release with the bundled acorn-jsx is appreciated. |
As explained in #159 (comment) rollup doesn't like half-internalized externals. This change will treat all dependencies as externals and thus won't try to convert their imports.
#164 implements step 1. |
I just released v0.19.6 hopefully fixing this for now. |
trying it now. |
it works 👍 i think I was testing it with the old build or something. |
also stumbled into this, I got around it by using |
Please follow #165 for moving the acorn dependencies out of the bundle :) |
#165 seems good to go, I'll merge and release it soon. |
* Upgrade webpack to latest, as seen in plotly/dash-component-boilerplate#12 * Upgrade react-live to latest, as seen in bublejs/buble#159 * Make lint happy again * Update all packages in yarn
I recently updated to the latest buble through react-live and my editor stopped working with the following error:
It seems that we are trying to use the default import from
acorn
that is gone.Later I get:
After inspecting, I realized that it is trying to read
tokTypes
fromacorn
.The text was updated successfully, but these errors were encountered: