Skip to content

Remove "type": "commonjs" from package.json - #556

Merged
gregtatum merged 1 commit into
projectfluent:masterfrom
Demivan:fix-webpack-build
Jun 1, 2021
Merged

gregtatum merged 1 commit into
projectfluent:masterfrom
Demivan:fix-webpack-build

Conversation

@Demivan

@Demivan Demivan commented May 28, 2021

Copy link
Copy Markdown
Contributor

Setting "type" field in package.json forces Webpack to treat all files in the package as either CommonJS or EcmaScript.
Without it Webpack correctly uses "main" and "module" fields.
Fixes #517

@Pike

Pike commented May 28, 2021

Copy link
Copy Markdown
Contributor

Brain dump for Greg:

I recall that the current setup was a compromise between supporting some set of nodejs versions with different capabilities.

We still have node 10 in our testing matrix, which we could drop now. We also don't seem to have any sorts of integration tests with common package managers or bundlers. That part is pretty manual testing, IIRC.

@Demivan

Demivan commented May 28, 2021

Copy link
Copy Markdown
Contributor Author

Just checked with bundlers: Webpack 5, parcel, rollup, vite - working fine. I'm using yarn as package manager - that is working too.
Node 10, 12, 14 all work.
Node 10 only works with old syntax like described in #482. But changing "type": "commonjs" does not affect this. So I'm not sure what "type": "commonjs" fixes.
Both 12 and 14 can use native ES imports without any issue, so looks like #482 is not relevant now.

@ranisalt

Copy link
Copy Markdown
Contributor

Node 10 has passed it's end-of-life, I don't think someone comfortable with using that old of a Node version would mind using an equally outdated version of a dependency

@gregtatum

Copy link
Copy Markdown
Member

image

Yeah, it seems like we should only support as far back as the LTS release.

@gregtatum
gregtatum merged commit c723a5d into projectfluent:master Jun 1, 2021

@gregtatum gregtatum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, forgot to hit approve on it before merging. Thanks for the fix!

@Demivan
Demivan deleted the fix-webpack-build branch June 1, 2021 15:51
@Siilwyn

Siilwyn commented Jun 4, 2021

Copy link
Copy Markdown

👋 would it be possible to publish this in a package update? This is blocking usage for me atm.

@julienw

julienw commented Jun 7, 2021

Copy link
Copy Markdown
Contributor

Thanks so much for this!

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

Successfully merging this pull request may close these issues.

Not able to compile with webpack 5

6 participants