evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
37.99k stars 1.14k forks source link

esbuild should add "assert" option to css imports #1871

Closed jogibear9988 closed 2 years ago

jogibear9988 commented 2 years ago

if a css file is imported, esbuild should fix the output, so it contains assert for css. see: https://web.dev/css-module-scripts/

jogibear9988 commented 2 years ago

https://github.com/microsoft/monaco-editor/issues/2839

jogibear9988 commented 2 years ago

https://github.com/microsoft/vscode/issues/139416

hyrious commented 2 years ago

Although import assertions seem to be a standard way to specify asset types, it may break many other bundlers (mainly webpack) at current time. We should see whether tsc/babel/webpack/rollup are going to adopt this behavior too.

jogibear9988 commented 2 years ago

webpack has examples with asserts on this page: https://webpack.js.org/loaders/css-loader/

jogibear9988 commented 2 years ago

also it could be deactivatable via option

jogibear9988 commented 2 years ago

rollup also works on this: https://github.com/rollup/rollup/issues/3799 https://github.com/rollup/rollup/pull/4267 babel seems to have support: https://github.com/babel/babel/pull/12139 typescript also has support merged: https://github.com/microsoft/TypeScript/issues/40694

evanw commented 2 years ago

Import assertions are already supported in esbuild (since version 0.11.22 which was released way back in May 2021). They will be copied over to the output as long as the import path is external and the configured target environment supports parsing import assertions (e.g. with --bundle --external:*.css --format=esm --target=esnext). But you have to write the import assertions in the source code yourself. It wouldn't be correct to add import assertions automatically because that would change the semantics of the code, which would be incorrect.

jogibear9988 commented 2 years ago

@evanw also other syntax features are transformned when using a different target, so why not imports? imports wich import "css" without asserts are invalid javascript, so why should setting "format=esm" return invalid javascript? most imports are still in the wrong format today, cause there was no standard yet when these were written. but the bundler should put out a valid "esm" library wich should be usable without a bundler. If not, the output format should not be titled as "esm" cause it is not useable as esm, it's only usable in bundlers. And not everyone does bundling. see also https://open-wc.org/guides/developing-components/going-buildless/

evanw commented 2 years ago

@evanw also other syntax features are transformned when using a different target, so why not imports?

But imports are transformed: if you target an older browser that can't understand assert, then the asserts are removed. This is consistent with the transformation of other features such as import.meta. However, adding import assertions changes the semantics of the code so it's not something that should be done automatically.

evanw commented 2 years ago

Closing as "won't fix" for the reason described above. Adding assert when it's not there changes the semantics of the code, and so shouldn't be done by esbuild. You can add it in the source code yourself if you need it. This syntax is already supported by esbuild (support for it was added almost a year ago), so you can already use it with esbuild.

jogibear9988 commented 1 year ago

But as stated here: https://github.com/evanw/esbuild/issues/2314 assert with type css does still not work (it does not im my tests). A import with assert type css should return a cssStylesheet object

evanw commented 1 year ago

Yes. See my comment in the link you posted: https://github.com/evanw/esbuild/issues/2314#issuecomment-1539264985.

jogibear9988 commented 1 year ago

Yes, but that a file is imported more than once with different attributes is a edge case wich mostly will never occure. The feature is in chrome nearly 2 years, the spec is already again at stage 3, so it could be used. Typescript will add it in the next version. So if you won't add it, maybe at least add the additional import parameters to the onResolve callback, so a plugin can use them

jogibear9988 commented 1 year ago

I'd like to be able to do something like this:

  let onResolvePlugin = {
      name: 'example',
      setup(build) {
          build.onResolve({ filter: /\.css/ }, async args => {
              if (args.kind === "import-statement" && args.pluginData != 'cssConstructableStylesheet') {
                  const { path, resolveDir, pluginData, namespace, kind } = args;
                  const result = await build.resolve(path, {
                      resolveDir,
                      pluginData,
                      kind,
                      namespace,
                      pluginData: 'cssConstructableStylesheet'
                  });
                  return { path: result.path, pluginData: 'cssConstructableStylesheet' }
              }
          });
      },
  }

  const cssConstructStylesheetPlugin = {
      name: 'css import assertions',
      setup(build) {
          build.onLoad({ filter: /\.css$/ }, async (args) => {
              if (args.pluginData == 'cssConstructableStylesheet') {
                  const css = await readFile(args.path, 'utf8');
                  const fixedCss = css.replaceAll('`','\\`') .replaceAll(/\\(?:[1-7][0-7]{0,2}|[0-7]{2,3})/gm, '${\'\\$&\'}');
                  const contents = `
          const styles = new CSSStyleSheet();
          styles.replaceSync(\`${fixedCss}\`);
          export default styles;`;
                  return { contents };
              }
          });
      }
  }

wich does not work yet, but hopefully this is possible (first hour i'm look into esbuild plugins please be patient). But at the onResolve state, I don't know if a assertion is present or not