openstyles / usercss-meta

Parse usercss styles supported by the Stylus userstyle manager
Other
16 stars 5 forks source link

Distribute to npm #5

Closed eight04 closed 6 years ago

eight04 commented 6 years ago

I've completed the test in #4. Now we just need a build script and distribute the package to npm.

I can make a PR to add rollup support, but I'm not sure how we are going to make a release. Do we have a travis account or maybe someone would make a manual release?

Mottie commented 6 years ago

Nice work!

I'm not sure it needs to be packaged up with rollup, but yes we should go ahead and publish to npm. Anyone else have any feedback about this?

And actually, now that the code only parses usercss metadata, should we rename the repo again?

eight04 commented 6 years ago

I'm not sure it needs to be packaged up with rollup

Since there is only one file (index.js), it could be converted to UMD as @silverwind mentioned in #1. The benefit of using a build tool:

the code only parses usercss metadata, should we rename the repo again?

Probably usercss-metadata-parser would be more accurate (is it too lengthy?). The search result of "userscript metadata" on npm.

Mottie commented 6 years ago

Since there is only one file (index.js)

I haven't used rollup that much but it seems to me that developers could create their own bundles from what we have already provided. I'm fine with whatever.

Probably usercss-metadata-parser

Would it be better to start the name with parser? It's not too lengthy if it is clear what the module does.

silverwind commented 6 years ago

Nice work. As for UMD vs. bundler, the only benefit I see is that dependencies would be much easier to integrate, but as the goal is to be dependency-less, I think UMD might be the right way.

You can still do minification without a bundler. If you take a look at this package by me, I solved it through a few make tasks, so for example, make min generates .min.js and then I have tasks like make patch to release a new patch-level version to npm. I can set something similar here if you approve.

As for a package name, I think I'd prefer just usercss with a method parse on the exported object. That way, we can extend it later with more methods and maybe make it a more general useful library.

Taking a quick glance at the code, I see node's url module is the last dependency. I think we can avoid bundling a dependency by either using a simple RegEx or alternatively do a dynamic import of either node's urlor URL in browsers, but as it's just used to parse the protocol from a URL, I think a Regex might just be good enough.

eight04 commented 6 years ago

make tasks

I think we can simply use npm scripts to run tasks, so we don't need an additional dev dependency and node_modules/.bin/ prefix. For example: https://github.com/eight04/webext-menus/blob/8efa65420ee0d2c879e61f9b165b6ebc40986a80/package.json#L8-L16

just usercss with a method parse

A reason to avoid usercss as package (module) name is that Stylus already took usercss variable name for usercss builder. How about usercss-meta? which is similar to userscript-meta.

Usage example:

const usercssMeta = require("usercss-meta");
const data = usercssMeta.parse("...");

const parser = usercssMeta.createParser({...});
const data = parser.parse("...");
silverwind commented 6 years ago

I think we can simply use npm scripts to run tasks

Yeah, that's fine with me too, its just that those npm script one-liners can get pretty unwieldly, that's why I prefer make.

Stylus already took usercss variable name for usercss builder

Is that really an issue? Variables can be renamed. Reason why I like usercss is because it's a simple short name and the de-facto implementation should come from it. As for a interface, I was thinking something similar to JSON:

const usercss = require('usercss')

const obj = usercss.parse(str, opts);
const str = usercss.stringify(obj, opts);
silverwind commented 6 years ago

Because I'm paranoid of someone taking the package name on npm, I went ahead and published a placeholder package at that name: https://www.npmjs.com/package/usercss

eight04 commented 6 years ago

Since our parser (this repo) can only parse the metadata part of usercss, I would suggest leaving usercss to someone who is going to implement a full parser, including @document rules and section splitting, with preprocessor support.

something similar to JSON

+1. userscript-meta has stringify method as well.

silverwind commented 6 years ago

I would suggest leaving usercss to someone who is going to implement a full parser

We (or I) can do that later. My idea is to not stop at just parsing the header because practically, the input for the parser will most likely be a whole file and it should be possible to round-trip it through parse and stringify.

For now, we can output the css body as a raw string in a property on the object, and maybe later add the parser for the @document sections.

eight04 commented 6 years ago

not stop at just parsing the header because practically, the input for the parser should be the whole file

I can see that it would get pretty complicated when preprocessor involves, and we have to parse CSS to extract @document rules. BTW, Stylus uses CSSLint as the CSS parser.

IMHO, a package should be small and simple. Even if we want to implement a full parser, we can still keep this package named usercss-meta which focuses on parsing the metadata (and maybe stringify), then create another package named usercss, which require('usercss-meta') and other CSS parsers. This also benefits users who only need the metadata parser.

stringify

If this is going to be implemented, I think we should keep the code in tree-shakable CommonJS/ES module so users who only need the parser (e.g. Stylus) can create the bundle without including the stringifier.

We can even put them in separated files:

index.js
lib/parse.js (current index.js)
lib/stringify.js
const {parse} = require('usercss-meta/lib/parse');
silverwind commented 6 years ago

I can see that it would get pretty complicated when preprocessor involves, and we have to parse CSS to extract @document rules. BTW, Stylus uses CSSLint as the CSS parser.

Yes, that will be nasty to parse, let's leave it out. What do you think of my idea of providing the css without the header in a css property, leaving the parsing up to the user?

If this is going to be implemented, I think we should keep the code in tree-shakable CommonJS/ES module so users who only need the parser (e.g. Stylus) can create the bundle without including the stringifier.

I think for completeness, we need to include stringify, probably in a separate file will be best, yeah. Thought I don't see much of an issue distributing a few kilobytes of dependency-less JavaScript to every user.

Mottie commented 6 years ago

I'd also vote to use usercss-meta for this module and only focus on parsing the meta data.

We can create another repository for usercss to implement a full parser and use this one as a dependency. I like the idea of having a full parser, but not everyone will need it; for example, @DecentM only needs to parse the meta data.

Hmm, now I'm starting to think we should make a new organization LOL.

eight04 commented 6 years ago

providing the css without the header in a css property

There are some issues for this:

Is CSS code part of the metadata?

But I like the idea. Maybe we can add a method usercssMeta.search(text) which would search the metadata block in the text, so users can use it like:

const usercss = require('usercss-meta');
const result = usercss.search(usercssCode);
assert.deepEqual(result, {
    start: ...,
    end: ...,
    match: "/* ==UserStyle== ... ==/UserStyle== */",
    beforeMatch: "...",
    afterMatch: "..."
});
const metadataObject = usercss.parse(result.match);
Mottie commented 6 years ago

Wouldn't the meta block always be at or near the top? And, the start and end userstyle tags may or may not contain whitespace, e.g. /*==UserStyle... or /*\n ==UserStyle, so the match should use a regular expression.

eight04 commented 6 years ago

It's just an example to show what the result returned bt search() would look like:

const usercss = require("usercss-meta");
const result = usercss.search(usercssCode);
/* => {
    start: 0, // the index of matched text
    end: 123, // the end index of matched text
    match: "/* ==UserStyle== ... ==/UserStyle== *\/", // matched text
    beforeMatch: "", // text before the match, same as `usercssCode.slice(0, result.start)`
    afterMatch: "..." // text after the match `usercssCode.slice(result.end)`
});
*/
const metadataObject = usercss.parse(result.match);
const cssCodeWithoutMeta = result.before + result.after;
silverwind commented 6 years ago

Maybe we can add a method usercssMeta.search(text)

Not sure I see the usefulness of search. I think we should only support the simple case of single metadata block at the start of the CSS. Anything else will only add complexity and I see no real benefit of having CSS before the block, or even multiple blocks.

Is CSS code part of the metadata?

I'd say no.

Should parse() return a [metadataObject, cssCode] tuple? Should stringify() accept a tuple too?

Why not a flat object like {metaField1, metaField2, css}? Unless you stop parsing at the end of the metadata block, you'll be reading the whole file, at which point, it doesn't really cost much to copy the css in.

I had an idea for the usercss format which would parse @document rules regardless if the brower supports it (through regex), which would allow Stylus to do away with the multiple style sections and the style could be a single file that can easily be copy-pasted. Basically like the Mozilla format, but with standard @document instead of @-moz-document. I think I'd like this parsing to be done in this module and with that, usercss might be the more fitting name.

eight04 commented 6 years ago

Not sure I see the usefulness of search

A search is required to find the end tag (==/UserStyle==, assuming the metadata block is always at the start of the string).

having CSS before the block, or even multiple blocks.

Actually, the metadata block is not forced to be at the top of the file. This allows authors to add comments before the metadata (e.g. license header and linter configuration). Userscript also allows metadata to be placed anywhere in the file.

Also, we are not going to allow multiple metadata blocks in usercss, but providing a simple function to return the position of the metadata block. This is handy to extract metadata block from a usercss file.

Is CSS code part of the metadata?

I'd say no.

Then we shouldn't put the CSS along with other meta. Also if they are put together, we have to face the problem that the field name may contain css:

const result = usercssMeta.parse(
  `
/* ==UserStyle==
@name test
@version 0.1.0
@namespace example.com
@css 3
==/UserStyle== */

body {
    color: red;
}
`,
  { unknownKey: "assign" }
);
console.log(result.css); // ?

it doesn't really cost much to copy the css in

It doesn't, but it is useless for

I'd prefer a search function so users can decide how to use them:

I had an idea for the usercss format which would parse @document rules regardless if the brower supports it (through regex), which would allow Stylus to do away with the multiple style sections and the style could be a single file that can easily be copy-pasted. Basically like the Mozilla format, but with standard @document instead of @-moz-document. I think I'd like this parsing to be done in this module and with that, usercss might be the more fitting name.

What is the difference comparing to current usercss? If it is just about the name of the at-rule (@document v.s. @-moz-document), I don't think it is a good idea since @-moz-document has been used for years. In fact, @-moz-document is supported by Firefox and no one supports @document.

eight04 commented 6 years ago

I pushed the package to NPM manually. @Mottie and @silverwind are invited as collaborators.

I also noticed that there are already 3 releases on Github (0.1.0...0.3.0), but the version in package.json is still 0.1.0. I bumped the version to 0.4.0.