Latest ESM package support breaks build with webpack

thomasWajs commented 4 years ago

Issue details

We upgraded to prosemirror-view v1.13.2, which brings "ESM package support" with commit https://github.com/ProseMirror/prosemirror-view/commit/127ce0a1a301a748e8e2140bce695e6a96b17b23. I'm definitely fuzzy around the whole js transpiling process, but this commit did broke our webpack/babel build and I'm not sure if it's intended or not.

Since upgrading, our webpack build will import content from the prosemirror-*/src directories, without transpiling it, instead of importing it from prosemirror-*/dist directories. This will in turn wreak havoc on IE browsers, for which ES support is rather... lacking.

It seems this is not unseen behaviour :

I have no clue if this is a matter of changing our webpack configuration, or if this is something prosemirror should address.

ProseMirror version

prosemirror-view 1.13.2

bZichett commented 4 years ago

I'm getting the same issue. Looks like the root cause may be webpack, but prosemirror libs might be able to refactored to dodge the bug? (Unless it's stemming from a dependency of prosemirror - I didnt check this)

thomasWajs commented 4 years ago

It seems there's a way to work around the issue by configuring resolve.mainFields as such : resolve: { mainFields: ["main", "module"] },

But it may break loading of other dependencies in the build.

thomasWajs commented 4 years ago

People over there got the exact same issue, and resolved it by adding a "browser" entry point that mimic "main" : https://github.com/lancedikson/bowser/issues/355

jljorgenson18 commented 4 years ago

I think adding a browser field makes the most sense. Otherwise, a lot of people are going to have to update their build configs to transpile prosemirror or adjust their mainFields. An ESM supported environment should end up looking for "module" so a "browser" field would allow build tools like Webpack and ESM to coexist.

marijnh commented 4 years ago

I'm in the process of releasing new versions of all packages that include a compiled-down ES module bundle, which should solve this.

Nico-Roesch commented 4 years ago

Hi Marjin, we still get a lot of build error messages with the newest packages:

ERROR in ./node_modules/prosemirror-view/dist/index.mjs 2436:35-48
Can't import the named export 'TextSelection' from non EcmaScript module (only default export is available)
 @ ./src/index.ts

We are using webpack to build the files and are developing using typescript

cmlenz commented 4 years ago

Adding the following rule to our Webpack configuration does fix the build errors:

  {
    test: /\.mjs$/,
    include: /node_modules/,
    type: 'javascript/auto'
  }

(From https://github.com/reactioncommerce/reaction-component-library/issues/399)

vahidov commented 4 years ago

react-scripts 3.2.0 error

/node_modules/prosemirror-commands/dist/index.mjs
Can't import the named export 'AllSelection' from non EcmaScript module (only default export is available)

Can't fix without eject.

marijnh commented 4 years ago

Any idea why your bundlers are not importing the ES module version of the code? Could it be that you have some stale version of prosemirror-model in your tree?

vahidov commented 4 years ago

@marijnh latest 1.8.1

prosemirror-model@^1.0.0, prosemirror-model@^1.1.0, prosemirror-model@^1.2.0:
  version "1.8.1"
  resolved "https://registry.yarnpkg.com/prosemirror-model/-/prosemirror-model-1.8.1.tgz#44669d6b9dc84a69e4a7b54b77f12905d2f55250"
  integrity sha512-mpXHJQ99NllmQ4Kz3PSTc1j79iiBqpJWWAib4OPiZUlYSvcj3+nolBbRFWUjpZ1MRELWAjHKPBEzzhQ4nB6vFg==
  dependencies:
    orderedmap "^1.1.0"

marijnh commented 4 years ago

Can someone show a minimal webpack config that reproduces the issue? I'm not very familiar with webpack, but when I simply run it on a file that imports from prosemirror-model I'm not seeing any issues.

vahidov commented 4 years ago

@marijnh

yarn create react-app testprosemirror
cd testprosemirror
yarn add prosemirror-state
edit src/index.js -> add import 'prosemirror-commands'
yarn start

get compile error

marijnh commented 4 years ago

Unfortunately, that works for me (assuming you mean prosemirror-state where you wrote prosemirror-commands).

Nico-Roesch commented 4 years ago

I got an error when I used the script above with the prosemirror-view component:

./node_modules/prosemirror-view/dist/index.mjs
Can't import the named export 'DOMParser' from non EcmaScript module (only default export is available)

With prosemirror-state I also had no errors

marijnh commented 4 years ago

Okay, the trick was to import something that depends on other prosemirror packages. The react-app cruft wasn't necessary.

It seems that Webpack will respect the module field for dependencies directly imported by the main script, but then when, from there, it imports another package, it'll use the main field instead. I do not know why, and it doesn't seem to make any sense, but running webpack with --verbose shows that it ends up looking at ./node_modules/prosemirror-model/dist/index.js when I import prosemirror-commands. I have no idea why it would act that way.

lcswillems commented 4 years ago

I opened an issue with a way to reproduce the current error: https://github.com/ProseMirror/prosemirror/issues/1008#issuecomment-555530051

The error I get is with prosemirror-inputrules.

marijnh commented 4 years ago

I guess we need someone with more knowledge of webpack's resolution to look at this. Either we're doing something unexpected in the way the prosemirror packages provide their ES modules, or (a lot less likely) we're somehow hitting an obscure webpack bug.

marijnh commented 4 years ago

(Rollup doesn't seem to have any issues with transitive prosemirror dependencies, for what it's worth.)

bZichett commented 4 years ago

It seems there's a way to work around the issue by configuring resolve.mainFields as such : resolve: { mainFields: ["main", "module"] },

But it may break loading of other dependencies in the build.

(@thomasWajs suggested this above)

But resolve.mainFields: ['browser', 'main', 'module'] is safer and works for me.

Ex: I had a dependency object-hash which required browser field resolution. (I started with Thomas' solution but changing the import to object-hash/dist/object_hash but this was not needed after applying the above since browser points to it.)

@marijnh + all: Webpack now defaults to dist/index.js in prosemirror bundles rather than dist/index.mjs. The default order for webpack is apparently module then main. Honestly, don't know much about mjs (only just read a little about it now) so it seems we're just dodging those files with this webpack config patch.

jljorgenson18 commented 4 years ago

The default order is different depending on your target. Browser will be the first choice for most use cases in Webpack

https://webpack.js.org/configuration/resolve/#resolvemainfields

When the target property is set to webworker, web, or left unspecified:

webpack.config.js
module.exports = {
  //...
  resolve: {
    mainFields: ['browser', 'module', 'main']
  }
};
For any other target (including node):

webpack.config.js
module.exports = {
  //...
  resolve: {
    mainFields: ['module', 'main']
  }
};

bZichett commented 4 years ago

Thanks for clarifying, @jljorgenson18

So, still, the override proposed by @thomasWajs, should be amended for web builds with browser otherwise things are more likely to break

marijnh commented 4 years ago

Adding a browser field that points at the CommonJS file would largely defeat the purpose of having an ES module at all (and would be a rather random workaround in any case). So I don't intend to do that, and would prefer to figure out why webpack is behaving the way it is here.

jljorgenson18 commented 4 years ago

Just to be clear, do you mean an actual CommonJs module or the compiled version that was exported before the ESM change? Is the goal that Webpack users will now import the mjs file by default and not the previous dist/index.js files?

bZichett commented 4 years ago

Adding a browser field that points at the CommonJS file would largely defeat the purpose of having an ES module at all (and would be a rather random workaround in any case). So I don't intend to do that, and would prefer to figure out why webpack is behaving the way it is here.

Of course, I just want to be able to build web bundles that include additional prosemirror bug fixes / updates until someone figures out root cause (if it's webpack core related, I have yet to even upgrade to webpack 4, so I'll be stuck for a while.)

marduke182 commented 4 years ago

Just to clarify, @marijnh Is there any strong opinion about using .mjs extension? If there is not, we can do this approach:

Build CommonJS and ESM files into dist/cjs/index.js and dist/esm/index.js

Change package.json to:

{
"main": "./dist/cjs/index.js" ,
"module": "./dist/esm/index.js" ,
}

marijnh commented 4 years ago

@marduke182 I tried that locally, but it didn't seem to resolve the webpack issue, so I concluded the extension isn't the problem. Do you have evidence that it is?

marduke182 commented 4 years ago

@marijnh mmm my evidence is based on the solution, for example adding this to Webpack works for us (https://bitbucket.org/atlassian/atlaskit-mk-2/pull-requests/7696).

 webpackConfig.module.rules.push({
    test: /\.mjs$/,
    include: /node_modules/,
    type: "javascript/auto"
  });

What this is doing is telling to Webpack, "Hey threat any mjs file as a normal Javascript file". The solution I proposed is our current approach to support ESM, I dont know why is not working to you :(.

marijnh commented 4 years ago

Ah, I must have done something wrong in my test—I just tried again and indeed, renaming those files to anything ending in .js works. That's kind of disappointing that, now that node is standardizing on .mjs, webpack still chokes on it, but oh well. I'll start updating the packages, again.

marduke182 commented 4 years ago

@marijnh yeap, It might be Webpack implementing an old version of mjs proposal, I remember reading about .mjs files not been compatible with commonjs ages ago. I just read it again and indead they are compatible.

marijnh commented 4 years ago

RIP our changelog.

The packages should all be updated now. Please see if things work better for you.

lcswillems commented 4 years ago

The standard is currently .mjs extension for ESM, but this will change with the new version of Node.js this week: adding "type": "module" in the package.json will allow .js files to be ESM. And I am pretty sure it will become the standard. In 5 years, all the .js files will be ESM, on server and on client.

marijnh commented 4 years ago

but this will change with the new version of Node.js

That's an additional mechanism, but .mjs will also continue to be used as a way to indicate that a file is an ES module.

joelpro2 commented 4 years ago

Any solution to this problem? I'm trying to use TipTap to create a rich-text editor in a VueJS project, but i have the same problem referenced here

ERROR in ./node_modules/prosemirror-view/dist/index.mjs 2436:35-48 Can't import the named export 'TextSelection' from non EcmaScript module (only default export is available)

As i'm using Vue Cli 3, i don't (and should not) have access to webpack raw configs. Meanwhile i have a property on my vue.config.js file called transpileDependencies to transpile a node-module dependency with Babel.

I've added this transpileDependencies: ['prosemirror-history', 'prosemirror-collab'] in my file but it still don't works

marijnh commented 4 years ago

Did you read the thread? Are you trying with the current versions?

Jaromudr commented 4 years ago

@marijnh

https://github.com/marijnh/rope-sequence Seems you still forgot to update rope-sequence repo.

SyntaxError: Unexpected token: keyword «const»
115530 | 
115531 | // CONCATENATED MODULE: ./app/vue-dashboard/node_modules/rope-sequence/dist/index.mjs
> 115532 | const GOOD_LEAF_SIZE = 200;
 115533 | 
 115534 | // :: class<T> A rope sequence is a persistent sequence data structure
 115535 | // that supports appending, prepending, and slicing without doing a

joelpro2 commented 4 years ago

Did you read the thread? Are you trying with the current versions?

yes i read, my tiptap version depends on prosemirror-view 1.13.2 so i guess

marijnh commented 4 years ago

Seems you still forgot to update rope-sequence repo.

Good point. Done with 1.3.1

marijnh commented 4 years ago

(w3c-keyname had the same issue)

marijnh commented 4 years ago

Err, rope-sequence 1.3.1 takes care of the .mjs file name, 1.3.2 also compiles the file down to ES5.

tsnobip commented 4 years ago

I think OrderedMap hasn't been updated, I have errors with webpack I don't have with Parcel for example so I guess it's related to this issue.

marijnh commented 4 years ago

Good point. I just tagged orderedmap 1.1.1

SalahAdDin commented 1 year ago

It is still happening to me:

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist/tiptap-core.cjs 7:24-53
Module not found: Error: Can't resolve 'prosemirror-keymap' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist'
 @ ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/react/dist/tiptap-react.cjs 7:11-34
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 35:0-42 139:17-26
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist/tiptap-core.cjs 10:26-57
Module not found: Error: Can't resolve 'prosemirror-commands' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist'
 @ ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/react/dist/tiptap-react.cjs 7:11-34
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 35:0-42 139:17-26
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist/tiptap-core.cjs 11:28-62
Module not found: Error: Can't resolve 'prosemirror-schema-list' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist'
 @ ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/react/dist/tiptap-react.cjs 7:11-34
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 35:0-42 139:17-26
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist/tiptap-core.esm.js 22:0-44
Module not found: Error: Can't resolve 'prosemirror-keymap' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist'
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 36:0-77 89:28-44
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist/tiptap-core.esm.js 25:0-590
Module not found: Error: Can't resolve 'prosemirror-commands' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist'
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 36:0-77 89:28-44
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist/tiptap-core.esm.js 26:0-133
Module not found: Error: Can't resolve 'prosemirror-schema-list' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/core/dist'
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 36:0-77 89:28-44
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/extension-gapcursor/dist/tiptap-extension-gapcursor.esm.js 2:0-50
Module not found: Error: Can't resolve 'prosemirror-gapcursor' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/extension-gapcursor/dist'
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 55:0-61 147:6-24
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/extension-history/dist/tiptap-extension-history.esm.js 2:0-58
Module not found: Error: Can't resolve 'prosemirror-history' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/extension-history/dist'
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 56:0-48 204:6-13
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

ERROR in ./src/plugins/strapi-tiptap-editor/node_modules/@tiptap/extension-table/dist/tiptap-extension-table.esm.js 2:0-280
Module not found: Error: Can't resolve '@tiptap/prosemirror-tables' in '/home/luisalaguna/Projects/Portfolio/backendv4/src/plugins/strapi-tiptap-editor/node_modules/@tiptap/extension-table/dist'
 @ ./src/plugins/strapi-tiptap-editor/admin/src/components/Wysiwyg/index.js 41:0-53 191:23-47
 @ ./src/plugins/strapi-tiptap-editor/admin/src/index.js 24:0-43 48:48-55
 @ ./src/plugins/strapi-tiptap-editor/strapi-admin.js 4:0-47
 @ ./.cache/admin/src/plugins.js 11:0-85 26:19-31
 @ ./.cache/admin/src/index.js 47:0-32 87:16-23

9 errors have detailed information that is not shown.
Use 'stats.errorDetails: true' resp. '--stats-error-details' to show it.

webpack 5.75.0 compiled with 9 errors in 52371 ms
No errors found.

ProseMirror / prosemirror

Latest ESM package support breaks build with webpack #1003

Issue details

ProseMirror version