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

Add support for browser bundle #4796

Closed
wants to merge 68 commits into from
Closed

Add support for browser bundle #4796

wants to merge 68 commits into from

Conversation

m-allanson
Copy link
Member

@m-allanson m-allanson commented May 21, 2020

Overview

This draft PR demonstrates how stylelint can publish a browser compatible bundle.

It:

  • adds a new entrypoint browser.js
  • adds a command to create a stylelint-esm.js bundle
  • adds a command to create separate bundles for each of stylelint's built-in syntaxes
  • modifies the API for the customSyntax option

This is a draft PR. You can use it to get an overview of how this functionality has been implemented. There may still be some rough edges.

Link to live demo (view source to see how it works): https://stylelint-browser-bundle.netlify.app

Refs: #3935

Tasks

View the Next Steps section to see completed and remaining work.

Example

Lint using the SCSS syntax:

<html>
  <head>
    <title>demo demo demo</title>
    <script type="module">
      import sl from "./stylelint-esm.js";
      import syntaxScss from "./syntax-scss.js";

      async function run() {
        const options = {
          code: `a.#{var} { color: #fff; }`,
          config: {
            rules: {
              "color-hex-length": "long"
            }
          },
          customSyntax: syntaxScss
        };

        const result = await sl.lint(options);
        console.log(result);
      }

      run();
    </script>
  </head>
  <body></body>
</html>

Note that the syntax option is not supported by this bundle. Syntaxes must be loaded externally and passed in using the customSyntax option. This allows syntaxes to be lazy-loaded as needed.

customSyntax

The customSyntax option which previously accepted a string is now overloaded to accept a string or a syntax object. This allows custom syntaxes to be loaded externally and then passed in to stylelint (see example above).

The problem

Stylelint was written for NodeJS using the CommonJS module format. This led to a couple of obstacles when attempting to bundle stylelint into a browser-friendly format.

Runtime module resolution

There are parts of stylelint that depend on require's module resolution algorithm to load modules at runtime.

For example, you can give stylelint a config like:

{
  "extends": "stylelint-config-standard",
  "rules": {
    "indentation": "tab",
    "number-leading-zero": null
  }
}

And it will load a module that matches the string identifier "stylelint-config-standard". There is no way for JS running in a browser to reliably resolve a string like "stylelint-config-standard" to the appropriate module.

Filesystem

Other parts of stylelint expect a filesystem to exist, to support things like caching results. There is no filesystem available when running in the browser.

Approach

To run stylelint in a browser, it needs to be converted to a different (browser-friendly) module format and it needs to run without access to a filesytem.

Note: there may be other approaches to doing this. I'd love to hear about them if so.

I aimed to create a new API that accepts a code string with a config object, and returns a stylelint result.

This would bypass stylelint's caching and loading functionality, avoiding most of stylelint's filesystem IO. This subset of stylelint should be much easier for a bundler to package up.

Refactoring

This is where most of the work was centered - separating the parts of stylelint that rely on filesystem access (caching, loading and extending configs, loading plugins, etc) from parts that do not (linting a code string with a config object)

Here's a diagram showing the dependencies in stylelint's lib directory, for the currently released version of stylelint:

stylelint-dep-chart-master

With the changes on this branch, the updated dependency diagram looks like this (new files highlighted purple):

stylelint-dep-chart-browser

browser.js is the new entry point. The remaining files are all extracted from their parent:

Source module Function extracted to
standalone.js getFormatterFunction.js
standalone.js --> prepareReturnValue.js
lintSource.js --> lintPostcssResult.js
createStylelintResult.js --> createPartialStylelintResult.js
augmentConfig.js --> normalizeAllRuleSettings.js

In most cases this involved copying an existing but unexported function out to a new 'child' module, then importing it back into its parent.


The result of these changes are that (if you squint a bit) you can see the lib directory has been split into halves.

The left half is concerned with all the filesystem things like caching, configs and plugins. Let's call this 'context'. The right half is more focussed on linting a string + config. We can call this 'core'.

Here's a wiggly line to demonstrate:

stylelint-dep-chart-divider

If you gloss over the exact file names, you might get to something like this:

stylelint-core-context

In the future, it may be useful to further refactor stylelint to make the context / core divide clearer.

Bundling

The bundling step was relatively straightforward once the refactoring had been done.

Using the new browser.js file as an entrypoint, Parcel creates an ES module bundle. Each of stylelint's syntaxes also gets a separate bundle, allowing syntaxes to be loaded on demand.

References: Prettier's browser bundle, ESLint's lintText method. Dependency diagrams created with Dependency Cruiser. Also thanks to @jeddy3 for regularly talking through ideas, approaches and the code.

Limitations

Does not support extends or plugins config keys

The browser bundle does not support the extends or plugins config keys. Here's a reminder of how these keys are processed by stylelint:

The value of "extends" is a "locater" (or an array of "locaters") that is ultimately require()d. It can fit whatever format works with Node's require.resolve() algorithm. That means a "locater" can be:

  • the name of a module in node_modules (e.g. stylelint-config-standard; that module's main file must be a valid JSON configuration)
  • an absolute path to a file (which makes sense if you're creating a JS object in a Node.js context and passing it in) with a .js or .json extension.
  • a relative path to a file with a .js or .json extension, relative to the referencing configuration (e.g. if configA has extends: "../configB", we'll look for configB relative to configA).

There is no file system available when running in a browser, so stylelint has no way to resolve the locators used in these keys.

A follow-up to this PR could add a plugin-loading API, and would allow this limitation to be revised.

formatters options is fixed to json

The browser bundle will always output results formatted with stylelint's JSON formatter.

Next Steps #

I'm going to split this PR into smaller review-friendly chunks. This has the added bonus of making it easier to track down any bugs or regressions.

Tasks:

  • merge separate refactoring PRs (see list below)
  • update customSyntax API to also accept a syntax object Accept syntax object in customSyntax option #4839
  • add new browser entry point (and tests)
  • add bundling script
  • add a smoke test for bundled code
  • output bundles as part of publish workflow
  • add docs
Expand this to see merged PR details

@m-allanson m-allanson changed the title Browser bundle WIP: Browser bundle May 21, 2020
@m-allanson m-allanson marked this pull request as draft May 21, 2020 12:33
@m-allanson m-allanson linked an issue May 28, 2020 that may be closed by this pull request
m-allanson added a commit that referenced this pull request May 28, 2020
In a future PR this module will be required by the new `browser.js` module. This change has been extracted from: #4796
m-allanson added a commit that referenced this pull request Jun 1, 2020
In a future PR this module will be required by the new `browser.js` module. This change has been extracted from: #4796
# By Mike Allanson (2) and others
* master:
  Fix changelog formatting
  A small refactor (#4837)
  Update CHANGELOG.md
  Add ignoreContextFunctionalPseudoClasses to selector-max-id (#4835)
This means no config is needed in `package.json`
# By Richard Hallows (4) and others
# Via GitHub
* master:
  Create codeql analysis workflow (#4850)
  Update CHANGELOG.md
  Add support for descriptions in stylelint command comments (#4848)
  Update CHANGELOG.md
  Add *-allowed-list/*-disallowed-list/*-required-list names for *-whitelist/*-blacklist/*-requirelist rules (#4845)
  Update CHANGELOG.md
  Add syntax object acceptance to customSyntax option (#4839)
  Bump typescript from 3.9.5 to 3.9.6 (#4853)
  Add ignoreComments[] to comment-empty-line-before (#4841)
  Bump autoprefixer from 9.8.0 to 9.8.2 (#4838)
* master:
  Update CHANGELOG.md
  Fix false negatives for where, is, nth-child and nth-last-child in selector-max-* (except selector-max-type) (#4842)
  Bump @types/lodash from 4.14.155 to 4.14.157 (#4869)
  Remove `postcss-reporter` package (#4858)
  Bump lodash from 4.17.15 to 4.17.19 (#4864)
  Replace 3rd-party type definitions (#4857)
* master: (34 commits)
  Update CHANGELOG.md
  Fix double-slash disable comments when followed by another comment (#4913)
  Update CHANGELOG.md (#4916)
  13.7.0
  Prepare 13.7.0
  Prepare changelog
  Update dependencies
  Update CHANGELOG.md
  Deprecate *-blacklist/*-requirelist/*-whitelist (#4892)
  Fix some path / glob problems (#4867)
  Update CHANGELOG.md
  Add a reportDescriptionlessDisables flag (#4907)
  Fix CHANGELOG.md format via Prettier (#4910)
  Fix callbacks in tests (#4903)
  Update CHANGELOG.md
  Fix false positives for trailing combinator in selector-combinator-space-after (#4878)
  Add coc-stylelint (#4901)
  Update CHANGELOG.md
  Add support for *.cjs config files (#4905)
  Add a reportDisables secondary option (#4897)
  ...
* master: (46 commits)
  Update CHANGELOG.md
  Add ignoreAtRules to property-no-unknown (#4965)
  Bump eslint from 7.11.0 to 7.12.1 (#5017)
  Bump typescript from 4.0.3 to 4.0.5 (#5016)
  Bump lint-staged from 10.4.0 to 10.5.1 (#5014)
  Bump remark-cli from 8.0.1 to 9.0.0 (#4996)
  Bump jest-circus from 26.5.3 to 26.6.1 (#5009)
  Bump got from 11.7.0 to 11.8.0 (#5007)
  Bump jest from 26.5.3 to 26.6.1 (#5008)
  Refactor formatter tests (#4988)
  Fix `isStandardSyntaxDeclaration.test.js` that use callbacks (#4972)
  Update CHANGELOG.md
  Add "comment-pattern" rule (#4962)
  Update CHANGELOG.md
  Show the pattern in "*-pattern" rule messages (#4975)
  Enable ESLint `no-shadow` and add disable comments (#4986)
  Report disables in the same manner as lints (#4973)
  Update dependencies (#4982)
  Fix some tests that use callbacks (#4970)
  Use own vendor utility instead of PostCSS (#4942) (#4963)
  ...

const bundleReport = require('@parcel/reporter-cli/lib/bundleReport').default;
const defaultConfigContents = require('@parcel/config-default');
const defaultConfigFilePath = require.resolve('@parcel/config-default');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just interesting, why parcel? rollup and webpack, even esbuild is better, parcel is very buggy

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the time I wrote this, Parcel looked like the simplest way to take CommonJS and output ESM. I didn't put much more thought into it than picking 'any' bundler to use for a proof of concept.

@Stvad
Copy link

Stvad commented Apr 6, 2021

👋 thanks for working on this! I was wondering what is the status of this PR?

@m-allanson
Copy link
Member Author

Hi @Stvad 👋

I lost a bit of steam with this. Though it's been on my mind again recently, as there's talk of a new major release for stylelint later this year.

A good chunk of the work from this PR has already been merged into stylelint. The biggest missing parts are:

  • the file core-entrypoint.js that exposes the new API
  • a bundling step that converts from CommonJS to ES Modules, and wraps everything together into a single file

(Note there may be some smaller tasks too, I'd need to dig back into the code to confirm.)

The bundling is where I ran out of steam. I don't think adding a bundler to stylelint is the right choice for the long-term maintenance of project. Currently you can run stylelint straight from GitHub, there's no bundling step at all. I think this approach has been a boon over the years as it's usually simpler to maintain and debug unbundled code.

However, if stylelint moves to ESM (see earlier link) then bundling could be much simpler (or unnecessary?) as the code will be in a browser-friendly module format already. No CommonJS to ESM conversion would be needed!

I think the "minimum" work needed to make this feature available would be something like:

  • merge core-entrypoint.js (and tests) into stylelint
  • wait for / work on issue #5205
  • document browser support as an experimental feature, and leave bundling (if needed) to stylelint users

I'd be interested to hear your thoughts on the above, and also how you'd like to use stylelint in a browser?

@Stvad
Copy link

Stvad commented Apr 8, 2021

@m-allanson thanks a lot for a detailed response!

a bundling step that converts from CommonJS to ES Modules, and wraps everything together into a single file

I'm curious why that needs to be a part of this project itself? I imagine as long as it's bundleable in principle - people can take a dependency on it and let webpack/etc do the job in the target project? (ah, I see you not is as an option later!)

I'd be interested to hear your thoughts on the above,

The proposal makes sense to me!

and also how you'd like to use stylelint in a browser?

My use-case is that I have a service that allows users to publish websites (https://roam.garden/) and one of the inputs I have is a custom CSS field.
I want to check the CSS validity at the upload/input time vs having a website build fail later becauese webpack does not like the presented CSS 😅.

So I don't need a linter exactly, but I need something that would allow me to check CSS for syntax errors and present results to a user on ~per line basis (via CodeMirror integration). StyleLint seemed like a good fit.

Upon finding out that I can't make it work - I'm trying my luck with https://github.com/csstree/validator but it seems to ignore some errors 🙁. Would appreciate suggestions on better ways of solving this! 🙂

@jeddy3 jeddy3 changed the title WIP: Browser bundle Add support for browser bundle Feb 10, 2022
@Mouvedia
Copy link
Contributor

@m-allanson for require.resolve you have import.meta.resolve.

@m-allanson
Copy link
Member Author

I'm closing this because it is very outdated and unlikely to be mergeable in the near future.

@m-allanson m-allanson closed this Dec 4, 2023
@ybiquitous ybiquitous deleted the browser-bundle branch January 20, 2024 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add support for running in a browser
4 participants