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

Strip preprocessor lang attribute from output #260

Open
firefish5000 opened this issue Sep 27, 2020 · 4 comments
Open

Strip preprocessor lang attribute from output #260

firefish5000 opened this issue Sep 27, 2020 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@firefish5000
Copy link

firefish5000 commented Sep 27, 2020

Is your feature request related to a problem? Please describe.
svelte-preprocess utilizes the lang attribute to identify the languages used, but leaves them in the output after preprocessing.

Describe the solution you'd like
I would like the lang attribute to be removed from the generated output, primarily for cleanliness.

How important is this feature to you?
Not important at all, but seems like something that should be done, and should be done here.

Additional context
Originally noticed by bluwy

@kaisermann kaisermann added enhancement New feature or request help wanted Extra attention is needed labels Sep 27, 2020
@kaisermann
Copy link
Member

Since we need to remove these attributes, we'll need to do it in the markup phase. This means we will probably have to capture and save somewhere the lang/type attributes in a markup preprocessor, check if there's an available preprocessor for the language and, if it does, remove the attribute. Then we would make the script/style preprocessor use the captured values instead of the ones passed in the attributes parameter. I think this is okay as we already have code to parse an attribute string (almost identical to the one in the compiler).

@firefish5000
Copy link
Author

Trying to think of issues, Only 2 concerns I see

  1. By stripping the lang attribute it will no longer be possible to detect that language in script/style preprocessors outside of us.
    Currently it is possible for multiple script/style preprocessors targeting the same lang to run in sequence. This will remove that possibility completly.
  2. Assuming our actual preprocessors run in script/style phase, we would be stripping the lang attribute before other markup preprocessors run, while keeping the content in that language. This makes it difficult for processors that run after us in the markup phase to determine the language the content is in and may cause them to behave unexpectedly.

Concern 1 is probably a non-issue as it probably isn't done often in practice. But it could be easily worked around by placing the other preprocessor inside svelte-preprocess.

Concern 2 is probably doesn't matter much in practice, but doesn't have an obvious solution. It is impossible for the preprocessor to know what language the code is currently in and if it can safely apply its modifications to it. Since I cannot imagine any benefit to having this done in separate steps anymore (Concern 1 removes all benefits I could foresee), and can conceive of issues that may occur because of it. I believe it would be better to alter them at the same time.

@firefish5000
Copy link
Author

firefish5000 commented Oct 23, 2020

We can solve number 2 by making this preprocessor terminal in the markup phase. That is, we preprocess the scripts/style and remove the lang tag during our markup phase. This can be easily done by parsing the markup (which we need to do to some extent anyways for aliases and template detection) or by calling svelte.preprocess internally (do this before stripping the lang tag to save a line of code).

If we do terminate at the markup phase. There is no real need to modify the existing preprocessors to run in markup phase either. We just need a wrapper.

function markupTerminator(langs: string[],preprocessors: PreprocessorGroup|PreprocessorGroup[]): PreprocessorGroup {
  return {
    async markup({ content ,filename }): Promise<Processed> {
      const preprocessed = svelte.preprocess(content ,preprocessors ,{ filename })
      // Insert magic-string code to strip lang tags that match langs string from preprocessed.code here
      return preprocessed
    }
  }
}
// Example usage
import {  typescript } from 'svelte-preprocess';
// Better names needed. Some name to distinguish it from the version that doesn't strip markup.
// Or possibly remove ability to run on script/style completely, and instead only offer versions that strip the markup
export function typescriptTerminalPreprocessor(config) {
  return markupTerminator(['ts','typescript','script/ts','script/typescript','text/typescript'], typescript(config))
}

Usage (naturally I do not recommend this name)

import {  typescriptTerminalPreprocessor } from 'svelte-preprocess';
module.exports = {
  "preprocess": typescriptTerminalPreprocessor({sourceMap: true})
}

@dummdidumm
Copy link
Member

Svelte 5 will support lang="ts" natively and also support types in the markup that way. Stripping lang="ts" could mean we end up in the weird situation where there's still TS in the markup (because svelte-preprocess et al don't handle that) but lang="ts" is removed, meaning the Svelte parser will choke. In this specific case we probably need to keep the lang tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants