Elderjs / elderjs

Elder.js is an opinionated static site generator and web framework for Svelte built with SEO in mind.
https://elderguide.com/tech/elderjs/
MIT License
2.11k stars 53 forks source link

cjs & esm support #224

Open btakita opened 2 years ago

btakita commented 2 years ago

The build & tests pass with the added esm support.

Followed the technique in https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1

Note this this will require a major version update as the package.json "exports" prop is used. In praxis, there will be no breaking changes unless a dependent imports from an internal module.

From https://github.com/Elderjs/elderjs/issues/216#issuecomment-934409397

Not possible with Elder.js current structure. Open to a PR to help resolve it, but currently don't have a business case for needing to upgrade to esm yet. In fact one of the core packages we rely on doesn't support esm yet so we'll have to wait a bit longer.

Not sure what core package does not support esm. The build & tests pass.

codecov-commenter commented 2 years ago

Codecov Report

Merging #224 (2cd015b) into master (bf528f1) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #224   +/-   ##
=======================================
  Coverage   82.14%   82.14%           
=======================================
  Files          46       46           
  Lines        1960     1960           
  Branches      465      440   -25     
=======================================
  Hits         1610     1610           
  Misses        348      348           
  Partials        2        2           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bf528f1...2cd015b. Read the comment docs.

nickreese commented 2 years ago

Thanks for working on this. 👍

The comment above about it not being possible, I may be wrong on as I may have conflated Elder.js with how we build Elder.js projects.

The core meaningful distinction I remember was that CJS allows the code to be executed before exporting the module. This is something we use aggressively on ElderGuide.com, FindEnergy.com, and other sites to get the most out of caching.

In looking through the Elder.js codebase quickly I couldn't find an example of this happening.

Package Updates

Thanks for taking a shot at this. I know there are some issues with specific versions of Svelte and it may be worth us pinning some of the other packages as they have been responsible for esoteric issues that are hard to debug.

My gut says we should do a @next release when this is ready and our team will test it on our internal projects to make sure nothing major is broken.

markjaquith commented 2 years ago

Is there any will to resurrect this and work towards getting it merged? I think ESM is the way forward, as increasingly packages are going ESM-only.

I was going to pull in remark-smartypants to do quote-curling in Markdown, but it's ESM only. I had to go resurrect a version of it from 2020, including pinning to an old version of retext. I anticipate this problem becoming worse over time.

Willing to help in any way I can.

nickreese commented 2 years ago

@markjaquith thanks for nudging this. Working on a major refactor of both the plugins and Elder.js core. It will be ESM and have full TS support.

Absolutely huge undertaking as this move required moving all tests to esm as well which jest doesnt support.

You can look at the Typescript branches in both projects… also the published beta on npm.

Probably 3-4 weeks out due to things going on in my personal life and needing to rework the default template.

The changes are almost big enough to warrant a 2.0 as there are a ton of other improvements as well.