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

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

Closed
jogibear9988 opened this issue Dec 20, 2021 · 14 comments
Closed

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

jogibear9988 opened this issue Dec 20, 2021 · 14 comments

Comments

@jogibear9988
Copy link

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
Copy link
Author

@jogibear9988
Copy link
Author

microsoft/vscode#139416

@hyrious
Copy link

hyrious commented Dec 20, 2021

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
Copy link
Author

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

@jogibear9988
Copy link
Author

also it could be deactivatable via option

@jogibear9988
Copy link
Author

rollup also works on this: rollup/rollup#3799 rollup/rollup#4267
babel seems to have support: babel/babel#12139
typescript also has support merged: microsoft/TypeScript#40694

@evanw
Copy link
Owner

evanw commented Dec 21, 2021

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
Copy link
Author

@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
Copy link
Owner

evanw commented Jan 22, 2022

@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
Copy link
Owner

evanw commented Apr 5, 2022

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.

@evanw evanw closed this as completed Apr 5, 2022
@jogibear9988
Copy link
Author

But as stated here: #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
Copy link
Owner

evanw commented Sep 13, 2023

Yes. See my comment in the link you posted: #2314 (comment).

@jogibear9988
Copy link
Author

jogibear9988 commented Sep 13, 2023

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
Copy link
Author

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

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

3 participants