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

reading-time v2 #38

Open
Josh-Cena opened this issue Aug 27, 2021 · 5 comments
Open

reading-time v2 #38

Josh-Cena opened this issue Aug 27, 2021 · 5 comments

Comments

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Aug 27, 2021

Here are a few changes we can make in v2 (most of which @ngryman has also mentioned), roughly in order of decreasing importance:

  1. Refactor the API to expose two separate functions;
  2. Remove text from the function return;
  3. Remove the badly named IOptions export;
  4. Migrate to TypeScript completely;
  5. Prefer ESM to CJS (once we have TypeScript this is just a matter of setting the transpilation target);
  6. Use a class-based implementation of readingTimeStream;
  7. Change the testing library from mocha to Jest.

@ngryman If you have limited availability, I can be here to help.

@ngryman
Copy link
Owner

ngryman commented Sep 3, 2021

All of this sounds great to me, thanks!

I finally moved so I should be more available starting next week. I should be able to help here.

Ref #18 for stuff related to the API changes.

@Josh-Cena
Copy link
Collaborator Author

Josh-Cena commented Sep 4, 2021

Number 2, 3 and 5 will be resolved in #40.

Number 6 will be resolved by #41.

@miguelcobain
Copy link

miguelcobain commented Oct 18, 2021

Just jumping in to say that, with my current setup, v1.4.0 worked fine without any changes, but updating to v1.5.0 required me to add the following to my webpack config:

node: {
  global: true
},
resolve: {
  fallback: {
    stream: 'stream-browserify'
  }
},
plugins: [
  new webpack.ProvidePlugin({
    process: 'process/browser',
    Buffer: ['buffer', 'Buffer']
  })
]

and had to install stream-browserify.

The problematic line was const Transform = require('stream').Transform, but this line is also present in v1.4.0. This is a node core package, so it is expected for some kind of polyfill to be needed, but for some strange reason, v1.4.0 just works without any particular configuration change. 🤷‍♂️

@sanderjson
Copy link

sanderjson commented Dec 15, 2021

Just jumping in to say that, with my current setup, v1.4.0 worked fine without any changes, but updating to v1.5.0 required me to add the following to my webpack config:]


Not sure if this is the same issue but I had problems with 1.5 on Vite. Using 1.4 for now.

util.inherits is not a function
TypeError: util.inherits is not a function

@Josh-Cena
Copy link
Collaborator Author

@sanderjson Yes, your problem is the same as the previous one. util is a Node library and is not polyfilled by Vite. You can either use 1.4, use 2.0.0-1 (which has a browser field in package.json), or import from reading-time/lib/reading-time instead.

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

No branches or pull requests

4 participants