GoogleChromeLabs / text-fragments-polyfill

Apache License 2.0
116 stars 27 forks source link

📦 Webpack 5 Compatibility :: "Package path ./dist/fragment-generation-utils.js is not exported from package" #95

Closed MikaelCarpenter closed 3 years ago

MikaelCarpenter commented 3 years ago

Context

I'm attempting to utilize this package and it's utilities to do operations with text fragments. I'm starting with a simple use case:

import { generateFragment } from 'text-fragments-polyfill/dist/fragment-generation-utils.js';

// ... 
// in some click handler
const fragment = generateFragment(window.getSelection())

Issue

In the simple use case above I get an error when building my project with Webpack 5:

ModuleNotFoundError: Module not found: Error: Package path ./dist/fragment-generation-utils.js is not 
exported from package /.../node_modules/text-fragments-polyfill (see exports field in 
/.../node_modules/text-fragments-polyfill/package.json)

Proposed Solution

Respecting the exports field in package.json was a requested feature that got implemented with Webpack 5. The documentation for this field can be seen here.

I have a PR incoming with the following adjustment:

Screen Shot 2021-11-10 at 1 09 06 PM

Additional Issue + Context

When testing this change, I was able to get rid of the "export" related error, but when building my project I was given a new error:

Can't resolve 'regenerator-runtime/runtime.js' in 'node_modules/text-fragments-polyfill/dist/text-fragments.js'

Looking into this, it appears to be an issue with polyfilling async/await. Now this might be a bridge too far, but I attempted a solution for this as well.

Proposed Solution

Here is a related StackOverflow post I used, which resulted in me creating a rollup.config.js and updating the prepare script to point to that config instead of inlining the rollup options.

After that change, I received the #warning-treating-module-as-external-dependency warning in reference to core-js so I also added that as a dev dependency so it can get bundled in as well.

The implications of this second solution:

I'm going to put up my PR with both of these fixes. But I understand this second part may be overkill from your perspective as the maintainers, so if you'd prefer another approach just let me know and I'll update the PR 👍

tomayac commented 3 years ago

Closed via #6.

tomayac commented 3 years ago

Just released v5.1.0 that includes this fix. Thank you!