Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Requesting ECMAScript module (ESM) bundle support #2033

Closed
endigo9740 opened this issue Jun 29, 2020 · 23 comments
Closed

Requesting ECMAScript module (ESM) bundle support #2033

endigo9740 opened this issue Jun 29, 2020 · 23 comments

Comments

@endigo9740
Copy link

endigo9740 commented Jun 29, 2020

EDIT: Please read and follow the instructions at https://docs.sheetjs.com/docs/getting-started/installation/frameworks before raising a new issue.

When this issue was raised, the module only shipped with a CommonJS / UMD build. Projects using Webpack or other bundlers used the default import syntax to pull the CommonJS build:

// old way
import XLSX from "xlsx";

v0.18.1 activated the ESM build for common tooling. See https://docs.sheetjs.com/docs/installation/frameworks for more details. The literal equivalent of the old import is

// new way
import * as XLSX from "xlsx";
import * as cptable from 'xlsx/dist/cpexcel.full.mjs';
XLSX.set_cptable(cptable);

Tree Shaking is supported with the ESM build!. If a project is only exporting XLSX files (using utils and writeFile), using named imports with writeFileXLSX drastically reduces bundle size:

import { utils, writeFile } from "xlsx";

Sheet.js is providing the following warning in the Angular CLI for the new v10 release:

Screen Shot 2020-06-29 at 12 12 16 PM

Starting with Angular 10 the Angular CLI now provide warnings for CommonJS modules. Read more about it here:
https://blog.angular.io/version-10-of-angular-now-available-78960babd41

When you use a dependency that is packaged with CommonJS, it can result in larger slower applications.
Starting with version 10, we now warn you when your build pulls in one of these bundles. If you’ve started seeing these warnings for your dependencies, let your dependency know that you’d prefer an ECMAScript module (ESM) bundle.

And here:
https://angular.io/guide/build#configuring-commonjs-dependencies

It is recommended that you avoid depending on CommonJS modules in your Angular applications. Depending on CommonJS modules can prevent bundlers and minifiers from optimizing your application, which results in larger bundle sizes. Instead, it is recommended that you use ECMAScript modules in your entire application. For more information, see How CommonJS is making your bundles larger

They reference the following article regarding the issues with CommonJS:
https://web.dev/commonjs-larger-bundles/

Additionally I've seen a few discussions by Angular staff members on Github discussing potentially dropping CommonJS support by default, and making it an opt-in feature in the future.

I realize the Angular team is taking a very strong armed approach here, but their reasoning seems solid. Needless to say I'd love to see ESM support if possible. Thanks!

@SheetJSDev
Copy link
Contributor

CommonJS requires no language changes and thus is compatible with older IE and other environments like PhotoShop extensions. ESM breaks older browsers and legacy environments with the introduction of the new export keyword. While Google and the Angular team may be quick to "move fast and break things", we'd prefer not to.

That said, is there some stopgap measure? Can we create a dummy index.mjs file that imports the standalone packaged dist script? We could also generate a script that just concatenates the dist script with "export default XLSX;"

@endigo9740
Copy link
Author

Yep, I understand the goal is wide support, but the stopgap solution you've suggested seems reasonable to me!

@itayperry
Copy link

Was this issue solved? I work with Angular as well :)

@srijken
Copy link

srijken commented Sep 21, 2020

@SheetJSDev what needs to be done to get any of those two stopgap measures done?

Would it be possible to publish the dist script concatenated with the default export as a separate package or..?

@Leccho
Copy link

Leccho commented Oct 28, 2020

I beleive you should still have a version with ECMAScript.

@techmaster242
Copy link

I'm running into this same issue. The resulting file is very large (725kB) and slow. If we could get this working as an ES module, it would help a lot.

@chacabuk
Copy link

chacabuk commented Jan 8, 2021

Any advance on this?

@sivadinesh1
Copy link

Team I get this warning, what should be done? no clear answer

@Leccho
Copy link

Leccho commented Mar 7, 2021

There's nothing to do as long as they don't update the librairie. It still works tought. It's just a warning to tell you it might take longer to load the app

@jlabanca
Copy link

Adding my vote.

As a sort of workaround, you can use lazy loading modules to at least prevent it from being include in the initial download.
https://angular.io/guide/lazy-loading-ngmodules

You need to make sure that any refences to XLSX are isolated to the lazily loaded module. You can use webpack-bundle-analyzer to analyze your app and see what's included in the main.js file.

@SheetJSDev
Copy link
Contributor

The write codecs and utility functions are easy to split, but the read codecs are harder. The whole point of read and readFile is to mask the file format details so it doesn't really make sense to say you only want XLSX read support or only want XLS read support. This is especially important since many third party tools "cheat" by exporting HTML tables or SpreadsheetML 2003 XML with a .xls extension.

It is theoretically possible to modularize as follows:

  1. split up each codec into its own module

  2. add some helper functions to populate the library with the codecs

  3. read / readFile / write / writeFile would only operate with the known codecs.

This would imply end-user code that looks like:

import { read as read_XLSX, write as write_XLSX } from 'xlsx/codecs/xlsx';
import { read as read_XLS } from 'xlsx/codecs/xls';
import { readFile, writeFile, read, write, load_read_codec, load_write_codec } from 'xlsx';

[read_XLSX, read_XLS].forEach(c => load_read_codec(c));
[write_XLSX].forEach(c => load_write_codec(c))

const good_wb = readFile("valid.xlsx"); // assuming valid.xlsx is a valid XLSX file, returns a workbook
try { 
  const bad_wb = readFile("invalid.htm"); // if the HTML codec is not loaded, throw an error
} catch(e) { }

XLSX.writeFile(good_wb, "out.xlsx"); // succeeds since XLSX write codec was loaded
try {
  XLSX.writeFile(good_wb, "out.xls"); // fails since XLS write codec was not loaded
} catch(e) { }

@blondie63
Copy link

Hi all, i've same issue on my angular 11 app, is there now a final solution ?

@Akxe
Copy link

Akxe commented Apr 30, 2021

import { read as read_XLSX, write as write_XLSX } from 'xlsx/codecs/xlsx';
import { read as read_XLS } from 'xlsx/codecs/xls';
import { readFile, writeFile, read, write, load_read_codec, load_write_codec } from 'xlsx';

[read_XLSX, read_XLS].forEach(c => load_read_codec(c));
[write_XLSX].forEach(c => load_write_codec(c))

The codec loading version is cool. It looks very similar to angular:

import { registerLocaleData } from '@angular/common';
import localeCs from '@angular/common/locales/cs';

registerLocaleData(localeCs);

If the error in the reading/writing file would have info about the missing codec, it could be loaded asychronousy.

async function writeFile(data, filename) {
  try {
    return XLSX.writeFile(data, filename);
  } catch(e) {
    if (e instance MissingCodecError) {
      // If we know that codec is missing try to load it
      return import(`xlsx/codecs/${e.missingCodec}`)
        // Install it
        .then(module => load_write_codec(module.default))
        // Then retry it
        .then(() => writeFile(data, filename));
    } else {
      // If it is some other error, just pass it out
      throw e;
    }
  }
}

@SheetJSDev
Copy link
Contributor

A rough cut of both Browser and NodeJS ESM has been committed and will be pushed in the next release.

@rg1220
Copy link

rg1220 commented Oct 23, 2021

I'm still seeing this warning in the latest release (0.17.3) when using Angular. Anything I should be doing differently than just using it directly (like below)?

import * as XLSX from 'xlsx';

@Injectable()
export class Test {
  download() {
    const wb: XLSX.WorkBook = XLSX.utils.book_new();
    const ws = XLSX.utils.aoa_to_sheet([ [1, 2, 3] ]);
    XLSX.utils.book_append_sheet(wb, ws, 'data');
    XLSX.writeFile(wb, 'file.xlsx');
  }
}

@SheetJSDev
Copy link
Contributor

The ESM builds use the .mjs extension. TypeScript recently added support for the extension, but since the latest version of angular still pins to typescript versions below 4.4.0 it is currently not possible to use it.

@AnthonyLenglet
Copy link

adding this as an (hopefully) final update to this for angular: the latest RC release for angular 14 supports typescript 4.7.0, which officially added support for module: 'node12'
https://github.com/angular/angular/releases/tag/14.0.0-next.16

@SheetJSDev
Copy link
Contributor

In 0.18.1 the module field was added to package.json. Testing against the angular2+ demo, ng is using the ESM build. This is readily tested in by importing only the version field. Using bundlejs.com to test, the actual size is roughly 5KB

@AnthonyLenglet
Copy link

AnthonyLenglet commented May 5, 2022

oh wow, wasn't aware about that change !

just tested with 0.18.5, the issue is definitely fixed in angular :D

0.17.4

Lazy Chunk Files              | Names                                  |  Raw Size | Estimated Transfer Size
86.8dd8a74f9ec15d8a.js        | view-view-module                       |   1.32 MB |               231.78 kB

0.18.5

Lazy Chunk Files              | Names                                  |  Raw Size | Estimated Transfer Size
46.2ffba5c4ce4d4d79.js        | view-view-module                       | 344.26 kB |                91.03 kB

@srl295
Copy link

srl295 commented Jul 8, 2022

If anyone else is coming here trying to understand how to migrate, this is what I had to change in unicode-org/cldr#2153 :

-import XLSX from "xlsx";
+import * as XLSX from "xlsx";

@SheetJSDev
Copy link
Contributor

The original issue has been updated with some comments and a link to the current documentation.

@trampboy
Copy link

trampboy commented Jan 9, 2023

@srl295
Hi Sri295, I do the same as you, but it not work for me, i do not know what is wrong with it.

version: 0.18.5
error:
image

@SheetJSDev
Copy link
Contributor

Please install as described in https://docs.sheetjs.com/docs/getting-started/installation/frameworks

Note that ViteJS support requires 0.18.10+ . Please raise a new issue if you encounter any problems.

@SheetJS SheetJS locked and limited conversation to collaborators Jan 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests