tfjs icon indicating copy to clipboard operation
tfjs copied to clipboard

Move tfjs-examples to ESM

Open paradite opened this issue 2 years ago • 2 comments

Please make sure that this is a feature request. As per our GitHub Policy, we only address code/doc bugs, performance issues, feature requests and build/installation issues on GitHub. tag:feature_template

System information

  • TensorFlow.js version (you are using): N.A.

Are you willing to contribute it (Yes/No):

Yes

Describe the feature and the current behavior/state.

Currently https://github.com/tensorflow/tfjs-examples has a mixture of commonJS style code and ESM style code:

  • const fs = require('fs');
  • const {loadJSON} = require('./util');
  • import * as fs from 'fs';
  • import * as tf from '@tensorflow/tfjs';

This used to work across browser and node when the js build tools are more lenient (babel 6 era).

However in 2022, many tools have stopped working unless you explicitly declare whether the project is commonJS or ESM.

For example, I tried to run lstm-text-generation and was forced to declare the project as ESM to make it work in browser (parcel automatically forced an upgrade from babel 6 to babel 7), and then migrate commonJS to ESM to make it work in node.

I think there is a need to move (gradually) the projects in tfjs-examples to ESM because tooling support has become more explicit in what they support and ESM is the official standard recognized by both browser and node. Also moving ESM has been the trend for the past 1-2 years and gaining momentum: https://github.com/sindresorhus/meta/discussions/15.

The first step would be to move how we run unit tests via global test_util.js. This is blocking people from moving individual demos to ESM because test_util.js uses require which is not allowed in ESM module.

A walkaround can be used to temporarily enable require syntax in ESM:

import { createRequire } from 'module';
const require = createRequire(import.meta.url);
const fs = require('fs');

I have used this technique in lstm-text-generation and was successful in moving the project to ESM for both browser and node, the only thing left is fixing the uni test because of the reason outlined above.

Will this change the current api? How?

It will not affect tfjs api, but will impact the code style for all projects in tfjs-examples.

Who will benefit with this feature?

People who try to run tfjs-examples locally, who are currently experiencing errors because old the tools (babel 6) stopped working and new tools requires explicit ESM or commonJS project, not a mixture of both.

Any Other info.

N.A.

paradite avatar May 29 '22 05:05 paradite

@paradite Thank you for pointing out this issue, I think it will be very beneficial to get the examples up to date. It will be greatly appreciated, if you can contribute to this effort. Thanks

pyu10055 avatar May 31 '22 15:05 pyu10055

@paradite Thank you for pointing out this issue, I think it will be very beneficial to get the examples up to date. It will be greatly appreciated, if you can contribute to this effort. Thanks

Thank you for the reply and go-ahead. I'll start submitting PRs.

paradite avatar May 31 '22 18:05 paradite

Hi, @paradite

Apologize for the delayed response and we are re-visiting our older feature requests and checking whether those feature requests implemented or not as of now and as you mentioned in one of the PR here, you'll start working once this PR #814 got merged if I'm not wrong so May I know are you working on this issue please?

If someone wants to contribute for this feature then you're always welcome and please feel free to do and please refer these links Ref-1Ref-2 . Thank you!

gaikwadrahul8 avatar May 09 '23 06:05 gaikwadrahul8

hi @gaikwadrahul8 sorry i wasn't able to find time to work on this.

paradite avatar May 09 '23 16:05 paradite