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

Lack of appropriate README #2

Open
propkitty opened this issue Feb 6, 2023 · 8 comments
Open

Lack of appropriate README #2

propkitty opened this issue Feb 6, 2023 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@propkitty
Copy link

This repo has been left all over the speed-measure-webpack-plugin issues, but there is little to no information on how this should be used when coming to the page. An improved README would go a long way to making it so people could appropriately test this possible successor.

@propkitty propkitty added the enhancement New feature or request label Feb 6, 2023
@ShuiRuTian
Copy link
Owner

ShuiRuTian commented Feb 6, 2023

Ah, I think there is a not bad README file in fact: https://github.com/ShuiRuTian/time-analytics-webpack-plugin/tree/main/packages/time-analytics-webpack-plugin

Could this README resolve your problem? If not, what else you think could make the readme better?

But you are right, maybe I need to make this README file more easy to access in the root page....

Why it's not in the root page:

The main purpose is to give a good README page in npm website, which is my prefer way when I want to read the info for a package.

@ShuiRuTian
Copy link
Owner

Will close this issue after one week if there is no response.

Thanks for the feedback!

@FBRAA
Copy link

FBRAA commented Feb 10, 2023

Hello, I think would be great to add the import statement of plugin, like
const { TimeAnalyticsPlugin } = require('time-analytics-webpack-plugin');
as it different from the origin plugin

@ShuiRuTian
Copy link
Owner

@FBRAA sorry, I could not fully get your point....

You mean we should provide mjs and cjs together? Or We should provide a default export?

@jackykwan-eventx
Copy link

jackykwan-eventx commented Feb 13, 2023

i think the Readme is not showing enough information
We can take a look at speed-measurement, they show:

  • How to install
  • How to import to webpack config
  • How to apply in existing code base (usage)

https://www.npmjs.com/package/speed-measure-webpack-plugin
image

Tbh, i can not find how to use this package in Readme

@FBRAA
Copy link

FBRAA commented Feb 13, 2023

@FBRAA sorry, I could not fully get your point....

You mean we should provide mjs and cjs together? Or We should provide a default export?

Readme should have information on how the import statement should be written in order your plugin to function.

As a fresh user, I firstly tried:

const TimeAnalytics = require('time-analytics-webpack-plugin'); 
const Plugin = TimeAnalytics();

it didnt work

then I tried

import TimeAnalytics from 'time-analytics-webpack-plugin'); 
const Plugin = TimeAnalytics();

didn't work also

then I tried using it as is, because TimeAnalytics() could not be function;

then I went to
const TimeAnalyticsPlugin = require('time-analytics-webpack-plugin');

console.logged it and got [Getter], which confused me,

then I had to go through files of your file, calculating more options on 'HOW TO IMPORT' topic,

finally somehow I reached this variant:

const { TimeAnalyticsPlugin } = require('time-analytics-webpack-plugin');

So, if there are options on how to import your plugin, they should be listed. If it is the only option, it should be written to README as well.
As far as I can see, most of plugins are imported to my project this way, and so was speed-measurement one. Maybe it would be good to make your way of import look same:
image

@ShuiRuTian
Copy link
Owner

@FBRAA Thanks for the detailed response!

You are right, I missed this part. The README is updated now. Please feel free to give more feedback if you think it's still not good enough.

But it's intentional to use "named export" rather than "default export". Although we only have one object exported.

The main motivation is to use IDE function -- auto complete. If a package uses default export, there is not an "official name" and you need to type a proper text as the name.

Like in

import TerserPlugin from 'terser-webpack-plugin'

you always need to type "TerserPlugin". And any other text is valid, it's just your choice.

But if a package use "named export", then you could trigger auto complete like

import {/* Trigger auto complete here */} from 'my-plugin' // in VSCode, use ctrl+space to trigger

The only valid "official name" could be used. If you want to rename the export object, you need as keyword.

@ShuiRuTian
Copy link
Owner

@jackykwan-eventx The README is updated.

Although I think we did have "How to use" section...

Any more feedback is welcomed if you think it's still not enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants