Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix language files for Node+ESM #1377

Merged
merged 26 commits into from
Nov 12, 2024
Merged

Fix language files for Node+ESM #1377

merged 26 commits into from
Nov 12, 2024

Conversation

sequba
Copy link
Contributor

@sequba sequba commented Feb 15, 2024

Context

  • changed babel configuration to produce the ESM build with .mjs extensions and adjust the import and export statements in the output (custom babel plugin)
  • added exports property to package.json that lists all valid ways of importing HyperFormula
  • added the migration guide

Useful reading about similar solutions:

Related issues:

Fixes #1344

New import paths for the language files

Module system valid import paths in HF 2.x valid import paths in HF 3.0.0
ES modules
  • hyperformula/es/i18n/languages/
  • hyperformula/es/i18n/languages/frFR
  • hyperformula/es/i18n/languages/frFR,js
  • hyperformula/i18n/languages/
  • hyperformula/i18n/languages/frFR
  • hyperformula/es/i18n/languages/ (legacy)
  • hyperformula/es/i18n/languages/frFR (legacy)
  • hyperformula/es/i18n/languages/frFR,js (legacy)
CommonJS
  • hyperformula/commonjs/i18n/languages/
  • hyperformula/commonjs/i18n/languages/frFR
  • hyperformula/commonjs/i18n/languages/frFR.js
  • hyperformula/i18n/languages/
  • hyperformula/i18n/languages/frFR
  • hyperformula/commonjs/i18n/languages/ (legacy)
  • hyperformula/commonjs/i18n/languages/frFR (legacy)
  • hyperformula/commonjs/i18n/languages/frFR.js (legacy)
UMD hyperformula/dist/languages/ hyperformula/dist/languages/

Breaking change for projects that use Typescript, Angular <=17, or Webpack <=4

Angular

Angular version supported by Angular legacy import paths new import paths
18 yes
17 yes ⚠️works after configuring moduleResolution ⚠️works after configuring moduleResolution
16 with Typescript 5 until 2024-11-08 ⚠️works after configuring moduleResolution ⚠️works after configuring moduleResolution
16 with Typescript 4 until 2024-11-08 ⚠️works after upgrading to typescript 5 and configuring moduleResolution ⚠️works after upgrading to typescript 5 and configuring moduleResolution
15 no

Typescript

Typescript version legacy import paths new import paths
5 ⚠️works after configuring moduleResolution ⚠️works after configuring moduleResolution
4 ⚠️works after configuring moduleResolution ⚠️works after configuring moduleResolution

Webpack

Webpack version legacy import paths new import paths
5
5 + babel
4 ⚠️works after configuring resolver-default
4 + babel ⚠️works after configuring resolver-default

Webpack 4 has 2 separate issues:

  1. It doesn't know how to handle mjs files.
  2. It doesn't support exports property to package.json.

For the former issue, there is a workaround. It's enough to configure a rule for mjs files. But it seems that there is no way to make it understand the exports property. For that reason, Webpack 4 can work with HyperFormula 3.0.0, but only with import paths that match the actual paths (ignoring the exports property that provides the artificial paths).

Further reading:

Parcel

Parcel version legacy import paths new import paths
<2.9.0
>=2.9.0 ⚠️works after configuring resolver-default

Further reading:

Testing

Test importing HF in various setups

  • ESM in node project
  • ESM in react project (with Typescript)
  • ESM in angular 18 project -> works with default configuration
  • ESM in angular 17 project -> works after configuring moduleResolution
  • ESM in angular 16 project (typescript 5) -> works after configuring moduleResolution
  • ESM in angular 16 project (typescript 4) -> works after upgrading to typescript 5 and configuring moduleResolution
  • ESM in webpack 5 project
  • ESM in webpack 5 project + babel
  • ESM in webpack 4 project -> works only with legacy paths and requires special configuration
  • ESM in webpack 4 project + babel -> works only with legacy paths and requires special configuration
  • ESM in webpack 5 project with Typescript 5 -> works after configuring moduleResolution
  • ESM in webpack 5 project with Typescript 4 -> works after configuring moduleResolution
  • ESM in parcel 2.11 project -> works after configuring resolver-default
  • ESM in parcel 2.8 project -> only legacy paths
  • CJS in node project
  • CJS in parcel 2.11 project
  • UMD in browser

Demo projects are available in the hyperformula-demos repo.

Tested by @budnix:

Vite ^5.1.4
Parcel2 ^2.0.0
Webpack5 ^5.8.0
Webpack4 ^4.42.0

Tested by @magierg

Webpack 5.9 + TS
React 18^ + TS
Angular 16.0.2 + TS 4.4.3
Angular 17.3.1 + TS 5.2
Svelte 3.5.4 + vite 4
Vue 3.3.4 + vite 4.4.9

Further verification

  • go through the angular setup and polish the guide
  • go through the typescript 4 and 5 setup and polish the guide
  • go through the webpack 4 and 5 setup and polish the guide
  • go through the parcel setup and polish the guide
  • go through all the notes and comments in the PR and make sure the guide is complete
  • validate by publint
  • test with https://arethetypeswrong.github.io/
  • CR
  • (@magierg) test the migration guide: create new projects and set them up using the instructions from the guide
  • (@magierg) final QA approval

Types of changes

  • Breaking change (a fix or a feature because of which an existing functionality doesn't work as expected anymore)
  • New feature or improvement (a non-breaking change that adds functionality)
  • Bug fix (a non-breaking change that fixes an issue)
  • Additional language file, or a change to an existing language file (translations)
  • Change to the documentation

Checklist:

  • I have reviewed the guidelines about Contributing to HyperFormula and I confirm that my code follows the code style of this project.
  • I have signed the Contributor License Agreement.
  • My change is compliant with the OpenDocument standard.
  • My change is compatible with Microsoft Excel.
  • My change is compatible with Google Sheets.
  • I described my changes in the CHANGELOG.md file.
  • My changes require a documentation update.
  • My changes require a migration guide.

@sequba sequba self-assigned this Feb 15, 2024
@sequba sequba linked an issue Feb 15, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Feb 15, 2024

Performance comparison of head (fecd03a) vs base (5def4ef)

                                     testName |   base |   head | change
------------------------------------------------------------------------
                                      Sheet A |  502.4 | 503.89 | +0.30%
                                      Sheet B | 161.16 | 161.44 | +0.17%
                                      Sheet T | 141.27 | 141.11 | -0.11%
                                Column ranges | 504.91 | 506.38 | +0.29%
Sheet A:  change value, add/remove row/column |  14.98 |  13.95 | -6.88%
 Sheet B: change value, add/remove row/column | 129.08 | 122.98 | -4.73%
                   Column ranges - add column | 151.87 | 147.88 | -2.63%
                Column ranges - without batch | 436.92 | 434.39 | -0.58%
                        Column ranges - batch | 110.77 | 111.05 | +0.25%

@sequba sequba marked this pull request as ready for review February 21, 2024 14:59
@sequba sequba requested a review from budnix February 21, 2024 15:08
Copy link
Member

@budnix budnix left a comment

Choose a reason for hiding this comment

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

I tested the dist files in some more popular bundlers and here are the results:

  • Vite ^5.1.4 - OK
  • Parcel2 ^2.0.0 - Does not support "exports" so the language file has to be imported with /es/ or /commonjs/ prefixes;
  • Webpack5 ^5.8.0 - OK
  • Webpack4 ^4.42.0 - I getting an error. Seems the webpack has the problem with importing values from "chevrotain".
// /node_modules/hyperformula/es/parser/FormulaParser.mjs
import { EmbeddedActionsParser, EMPTY_ALT, Lexer, tokenMatcher } from 'chevrotain';
// EmbeddedActionsParser, EMPTY_ALT and others are missing
ERROR in ./node_modules/hyperformula/es/parser/FormulaParser.mjs 689:125-137
Can't import the named export 'tokenMatcher' from non EcmaScript module (only default export is available)
 @ ./node_modules/hyperformula/es/parser/index.mjs
 @ ./node_modules/hyperformula/es/ArraySize.mjs
 @ ./node_modules/hyperformula/es/index.mjs
 @ ./src/index.js

ERROR in ./node_modules/hyperformula/es/parser/FormulaParser.mjs 708:29-41
Can't import the named export 'tokenMatcher' from non EcmaScript module (only default export is available)
 @ ./node_modules/hyperformula/es/parser/index.mjs
 @ ./node_modules/hyperformula/es/ArraySize.mjs
 @ ./node_modules/hyperformula/es/index.mjs
 @ ./src/index.js

package.json Outdated Show resolved Hide resolved
@sequba
Copy link
Contributor Author

sequba commented Feb 29, 2024

  • Webpack4 ^4.42.0 - I getting an error. Seems the webpack has the problem with importing values from "chevrotain".
// /node_modules/hyperformula/es/parser/FormulaParser.mjs
import { EmbeddedActionsParser, EMPTY_ALT, Lexer, tokenMatcher } from 'chevrotain';
// EmbeddedActionsParser, EMPTY_ALT and others are missing
ERROR in ./node_modules/hyperformula/es/parser/FormulaParser.mjs 689:125-137
Can't import the named export 'tokenMatcher' from non EcmaScript module (only default export is available)
 @ ./node_modules/hyperformula/es/parser/index.mjs
 @ ./node_modules/hyperformula/es/ArraySize.mjs
 @ ./node_modules/hyperformula/es/index.mjs
 @ ./src/index.js

ERROR in ./node_modules/hyperformula/es/parser/FormulaParser.mjs 708:29-41
Can't import the named export 'tokenMatcher' from non EcmaScript module (only default export is available)
 @ ./node_modules/hyperformula/es/parser/index.mjs
 @ ./node_modules/hyperformula/es/ArraySize.mjs
 @ ./node_modules/hyperformula/es/index.mjs
 @ ./src/index.js

I found out that webpack 4 does not support exports, and there seems to be no easy workaround for that. By tweaking the webpack configuration, I managed to make the project import HyperFormula correctly using the legacy paths. Working demo.

@sequba sequba requested a review from budnix February 29, 2024 15:52
@sequba sequba added the Breaking Change Will need a major release label Mar 5, 2024
@sequba sequba changed the title Fix language files for Node+ESM [DO NOT MERGE] Fix language files for Node+ESM Mar 5, 2024
@sequba sequba changed the title [DO NOT MERGE] Fix language files for Node+ESM [DO NOT MERGE!] Fix language files for Node+ESM Mar 5, 2024
@magierg
Copy link
Contributor

magierg commented Mar 27, 2024

I have retested all the demos available on https://github.com/handsontable/hyperformula-demos/tree/import-demos
image

plus the bundlers mentioned by @budnix getting the same results.

On top of the above I have tested:

  • Webpack 5.9 + TS - tsconfig.js setting change required:
"module": "node16",
"moduleResolution": "node16",
  • React 18^ + TS - same tsconfig.js change required
  • Angular 16.0.2 + TS 4.4.3 - @sequba to investigate tsconfig changes required
  • Angular 17.3.1 + TS 5.2 - same issue with TS
  • Svelte 3.5.4 + vite 4 - getting this error - might be user error - @sequba to investigate
Error: Language already registered.
    at HyperFormula.registerLanguage (/Users/magierg/repos/hyperformula-demos/svelte-demo/node_modules/hyperformula/commonjs/HyperFormula.js:287:13)
    at Hyperformula.svelte:18:15
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1892:22)
    at eval (/src/routes/+page.svelte:24:148)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1892:22)
    at Object.default (root.svelte:41:38)
    at eval (/node_modules/@sveltejs/kit/src/runtime/components/layout.svelte:8:41)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1892:22)
    at root.svelte:40:37
    at $$render (/node_modules/svelte/internal/index.mjs:1892:22)
  • Vue 3.3.4 + vite 4.4.9 - OK

@sequba
Copy link
Contributor Author

sequba commented Apr 12, 2024

TODO: verify it with the code example in issue #1143

@sequba
Copy link
Contributor Author

sequba commented Apr 25, 2024

Svelte 3.5.4 + vite 4 - getting this error - might be user error - @sequba to investigate

It works correctly. The issue you reported was caused by running the setup code twice. I created a demo for svelte: https://github.com/handsontable/hyperformula-demos/tree/import-demos/import-demo-esm-svelte

Angular case is still to be verified.

Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.36%. Comparing base (e32e132) to head (0cd4e05).
Report is 33 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1377   +/-   ##
========================================
  Coverage    97.36%   97.36%           
========================================
  Files          169      169           
  Lines        14421    14421           
  Branches      3021     3021           
========================================
  Hits         14041    14041           
  Misses         380      380           

@sequba
Copy link
Contributor Author

sequba commented Aug 8, 2024

Angular 16.0.2 + TS 4.4.3 - @sequba to investigate tsconfig changes required
Angular 17.3.1 + TS 5.2 - same issue with TS

Angular issue can be resolved by setting "moduleResolution": "bundler" in tsconfig.json. Tested with Angular 16, 17 and 18 and Typescript 5. Demo

Unfortunately, I couldn't make combination Angular + Typescript 4 work. I'm afraid we need to ask our clients to upgrade to Typescript 5 if they want to use HyperFormula 3.x with Angular.

@sequba sequba requested a review from budnix October 31, 2024 14:34
@sequba sequba changed the title [DO NOT MERGE!] Fix language files for Node+ESM Fix language files for Node+ESM Nov 5, 2024
Copy link
Member

@budnix budnix left a comment

Choose a reason for hiding this comment

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

Looks good 👌

@magierg
Copy link
Contributor

magierg commented Nov 8, 2024

I have retested the previously mentioned configurations and performed the migration steps - found out that the @babel/core is not required - this has been already addressed by @sequba

@sequba sequba merged commit c27a9d5 into develop Nov 12, 2024
21 of 22 checks passed
@sequba sequba deleted the feature/issue-1344 branch November 12, 2024 11:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Breaking Change Will need a major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Language files do not work with ES modules in node
3 participants