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

Initial Commit #1

Open
Mottie opened this issue Jan 8, 2018 · 12 comments
Open

Initial Commit #1

Mottie opened this issue Jan 8, 2018 · 12 comments

Comments

@Mottie
Copy link
Member

Mottie commented Jan 8, 2018

  • This first version only supports parsing the metadata block of usercss styles.
  • It has not yet been published to npm - we can do it once everyone feels that it's ready.
  • Collaborators please review and feel free to make any modifications as needed.
@eight04
Copy link
Collaborator

eight04 commented Jan 8, 2018

It's nice to split usercss parser out from the extension so other people can reuse it. However, I'm not sure what is the purpose of this library. If it is intended to be used by others, it shouldn't include such heavy dependencies like accord and stylus.

If this library is going to be the "usercss core" part of Stylus extension, I think we can split it into two different libraries:

  1. The metadata parser, including color parser (and maybe semver). If someone just need to parse the metadata, they can use this library.

  2. The usercss library itself, which depends on metadata parser. Users can extend it with multiple preprocessor, for example:

    const {Usercss} = require("usercss"); // remove "-parser" suffix
    const usercss = new Usercss;
    usercss.addPreprocessor("stylus", {
    	preprocess: (source, variables) => {...}
    });
    
    let userstyle = usercss.parseMeta(usercssSourceCode);
    // ... do something to userstyle.usercssData ...
    userstyle = await usercss.buildSections(userstyle); // or usercss.buildCode
    userstyle.sections.forEach(s => console.log(s.code)); // read-to-use CSS code

If we are just going to pull out the parser part of usercss, we can remove everything related to buildCode().

semver dependency should be removed since it is only used for validation. We can use regexp instead.

@Mottie
Copy link
Member Author

Mottie commented Jan 8, 2018

My original idea was to split out the full parser, but since it's integrated with CSSLint, I switched to only parsing the metadata... I just removed accord and stylus as a dependency. Maybe we can think about building a full parser in the future.

@Mottie
Copy link
Member Author

Mottie commented Jan 8, 2018

semver dependency should be removed since it is only used for validation. We can use regexp instead.

That's true too! 😸

@Mottie
Copy link
Member Author

Mottie commented Jan 8, 2018

I didn't want to modify the usercss too much to make it easier to keep up-to-date with the main file in the Stylus repo. Maybe we should forget about that limitation and clean up the unnecessary code.

@eight04
Copy link
Collaborator

eight04 commented Jan 8, 2018

make it easier to keep up-to-date with the main file in the Stylus repo

It might be better if Stylus can use this library. E.g.

// stylus/js/usercss.js
const {Parser} = require("parse-usercss");
const parser = new Parser;
// override builtin validators
parser.onValidate("url", (key, value) => {
	// custom validator for URLs
});
parser.onValidate("version", (key, value) => {
	// stylus uses semver to validate version
});
parser.onValidateVar("color", (key, type, label, value) => {
	// custom validator for colors
});
parser.on("end", meta => {
	// override null-check for meta.version, meta.namespace, and meta.name
});

// in buildMeta()
const meta = parser.parse(metadataSourceCode);
style.usercssData = meta;
...

I think @tophf can tell which APIs are needed.

@tophf
Copy link
Member

tophf commented Jan 9, 2018

Well, like I said, I'm not interested in developing external tools, but I agree it'd be nice if you end up with something that may be usable in Stylus as-is. All Stylus needs is for the parser to produce sections and errors, see moz-parser.js:78. The errors do not necessarily prevent the output of sections, at least Stylus allows saving and live-reloading of usercss with syntax errors, the errors being separately displayed (in addition to the lint report).

Ah, and your parser should be really fast because currently the new reusable cache in csslint produces the mozparser's output in just a few milliseconds on subsequent saving while the user edits the style. This is very important especially for the, hopefully, upcoming [automatic] preview-while-editing feature.

P.S. for the meta block, I don't know, simply return the parsed data and the errors array.

@silverwind
Copy link
Contributor

If we can achieve a dependency-free parser, integration into stylus could be as simple as adding a git submodule that points to this repo. That way you wouldn't even need a build step in stylus, @tophf. Thought, I'd certainly welcome if stylus would depend on this module using package.json.

@tophf
Copy link
Member

tophf commented Jan 12, 2018

On second thought, I don't see it as being useful in Stylus because Stylus needs CSSLint to show meaningful error messages when trying to install/save a broken style. A stand-alone simple parser serves another purpose.

@silverwind
Copy link
Contributor

On second thought, I don't see it as being useful in Stylus because Stylus needs CSSLint to show meaningful error messages when trying to install/save a broken style. A stand-alone simple parser serves another purpose.

I have to object. I think the concerns of parsing the usercss header and linting CSS should be separated, unless the parsers can output a lintable format which pretty much means it must output a AST, the basis of all modern linters.

@tophf
Copy link
Member

tophf commented Jan 12, 2018

Well, it's not about semantics or idealistic should-be's, it's about practical usefulness. The fact is, the current implementation in Stylus is useful as is, whereas a lint-less parser is not as it would require an additional linting pass, which is a waste of resources and time.

@eight04
Copy link
Collaborator

eight04 commented Jan 13, 2018

Currently, Stylus processes usercss in following steps:

  1. Parse usercss metadata. Extract information like name, variables, preprocessor, etc.
  2. Variables might be changed by users.
  3. If there is a preprocessor, run the preprocessor (e.g. compile stylus-lang into CSS).
  4. Parse the CSS code with CSSLint. Extract @document rules and split the code into multiple sections.
  5. If there is a postprocessor, run the postprocessor (e.g. insert CSS variables to each section).

If we make this library as a simple metadata parser, Stylus can use it in the first step and others can also use it to extract metadata information.

I agree that it is not very useful to include entire parser/compiler. The compiler would probably only be used by extensions (e.g. Stylus).


integration into stylus could be as simple as adding a git submodule that points to this repo

Stylus keeps libraries in the repo. We just have to browserify/rollup the module to a single dist file.

@silverwind
Copy link
Contributor

silverwind commented Jan 14, 2018

Stylus keeps libraries in the repo. We just have to browserify/rollup the module to a single dist file.

Yeah, or keep the parser in ES5 and use a UMD header so it's importable everywhere.

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