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 preloaders option #165

Closed
wants to merge 1 commit into from
Closed

add preloaders option #165

wants to merge 1 commit into from

Conversation

Sanshain
Copy link

@Sanshain Sanshain commented Jan 28, 2023

There are loaders built into esbuild that cannot be controlled (unlike loader plugins, no options can be specified), in some cases their default behavior may not work as expected, and requires some preparation of the source code before this code is processed appropriate loader.

it would seem that this is a mere trifle. After all, there is probably an option that would disable the processing of such links at the time of building, putting this part at the discretion of the developer. But the loader did not have such an option. Just as there is no option to manually set the root directory to search for the images.

It would seem like a good idea for such a narrow purpose to write an independent plugin that would deal with the processing of absolute paths in styles. But as it turned out, esbuild does not provide chain passing of execution from plugin to plugin, and thus processed files after processing went to the loader, and not to the svelte-esbuild plugin, and this completely spoiled the whole game.

And it seems to me that since it is impossible to influence loaders and introduce such functionality using plugins, it would be nice to make in such cases in the svelte-esbuild plugin itself the possibility of pre-processing the generated sources before they get processed by the corresponding loaders.

So, I suggest the following usage:

esbuildSvelte({
   preprocess: sveltePreprocess(),            
   compilerOptions: {},
   preloaders: {
      css: (content, opts) =>{
         // changes absolute paths to relative
         const code = content.replace(/(\/static\/images)/g, function handleReplace(match) {
       
            let dirname = path.dirname(process.argv[1])
            let relPath = path.relative(opts.path, dirname + '/scripts') + '/images'                                 
            return relPath;
         });         
         return code;         
      }
   }
})

For example the above code changes all paths with /static/images to relative path depending on its location position

@EMH333
Copy link
Owner

EMH333 commented Jan 28, 2023

Interesting! Note that Svelte preprocessors do chain (see https://svelte.dev/docs#compile-time-svelte-preprocess). Any reason you couldn't do this?:

const replaceProcessor = {
  style: ({content, filename}) => {
     // changes absolute paths to relative
     const code = content.replace(/(\/static\/images)/g, function handleReplace(match) {
   
        let dirname = path.dirname(process.argv[1])
        let relPath = path.relative(filename, dirname + '/scripts') + '/images'                                 
        return relPath;
     });         
     return {code};         
  }
}

esbuildSvelte({
   preprocess: [replaceProcessor, sveltePreprocess()],
})

I think this could potentially solve your issue, without adding additional complexity to this plugin which I'll admit I'm hesitant to do.

If this doesn't solve your issue, we can talk further. I'd ask that you fix the linting issues and add at least a basic test before I merge this. I'm wondering if providing control of the loader used for css could also be a simpler solution to this...

@Sanshain
Copy link
Author

Note that Svelte preprocessors do chain

It is wonderful. Indeed, your solution looks more competent.

I think this could potentially solve your issue, without adding additional complexity to this plugin which I'll admit I'm hesitant to do.

Apparently, there is no reason to make the plugin more difficult since this can be done through a custom preprocessor

@EMH333
Copy link
Owner

EMH333 commented Jan 29, 2023

Excellent! Glad to hear it. I'll go ahead and close this PR then.

@EMH333 EMH333 closed this Jan 29, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants