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

inject custom content script from Popup.tsx #329

Closed
wants to merge 8 commits into from

Conversation

lealahm
Copy link

@lealahm lealahm commented Dec 11, 2023

In the above code, I try adding a custom content script folder apart from the default content folder, which would be the code that can be injected on the page when the user clicks on a button inside popup.tsx or using background script.

@lealahm lealahm requested a review from Jonghakseo as a code owner December 11, 2023 11:34
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. We will check and reply to you as soon as possible.

Copy link
Collaborator

@PatrykKuniczak PatrykKuniczak left a comment

Choose a reason for hiding this comment

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

I don't think this is good idea to create duplicated content script.

I mean create step by step instruction, where you describe how to use this, dev which want to implement this, can copy content script, describe only how to attach this to project, like you create in this PR, and describe how to use this excecuteScript :)

src/pages/popup/Popup.tsx Show resolved Hide resolved
vite.config.ts Outdated Show resolved Hide resolved
src/pages/injectedContent/ui/index.tsx Outdated Show resolved Hide resolved
refreshOnUpdate('pages/content');

const root = document.createElement('div');
root.id = 'chrome-extension-boilerplate-react-vite-content-view-root';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
root.id = 'chrome-extension-boilerplate-react-vite-content-view-root';
root.id = 'chrome-extension-boilerplate-react-vite-injected-content-view-root';

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lealahm Why did you don't commit this suggestion?

src/pages/injectedContent/index.ts Outdated Show resolved Hide resolved
src/pages/injectedContent/ui/app.test.tsx Outdated Show resolved Hide resolved
src/pages/injectedContent/ui/app.tsx Outdated Show resolved Hide resolved
src/pages/injectedContent/ui/app.tsx Outdated Show resolved Hide resolved
src/pages/popup/Popup.tsx Outdated Show resolved Hide resolved
@romaindequidt
Copy link
Contributor

I try to use your branch and had to following erreur as soon as content script is run at the document start :
image
Have you managed to make it work on your side?

@romaindequidt
Copy link
Contributor

May you relate this pull request to its issue #306 , please ? Thanks,

@lealahm
Copy link
Author

lealahm commented Dec 30, 2023

@romaindequidt, Hello, I was also facing the same problem once I tested this out and still trying to solve this problem

@PatrykKuniczak PatrykKuniczak linked an issue Jan 10, 2024 that may be closed by this pull request
@PatrykKuniczak
Copy link
Collaborator

@lealahm any moves forward?

@tngflx
Copy link

tngflx commented Jan 24, 2024

@lealahm @romaindequidt the latest vite preload plugin already solved this problem, 685da06

@aspiers
Copy link
Contributor

aspiers commented Jan 28, 2024

I don't think these changes are enough; I was doing something very similar but I also needed to change this line:

In the case of this PR, it would need to be:

 if (!/content|injectedContent/.test(chunk.fileName)) {

const injectContentScript = async () => {
const [tab] = await chrome.tabs.query({ currentWindow: true, active: true });
console.log(tab.id)
chrome.scripting.executeScript({
Copy link
Contributor

Choose a reason for hiding this comment

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

This results in

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'executeScript')

I think you need to have the scripting permission in the manifest. You also need to have the activeTab permission, or have the content script allowed as described here:

https://developer.chrome.com/docs/extensions/reference/api/scripting

@enmanuelmag
Copy link

enmanuelmag commented Jan 29, 2024

I try to use your branch and had to following erreur as soon as content script is run at the document start : image Have you managed to make it work on your side?

I'm facing the same problem, it's explained in this issue, this PR will solve the problem? I want to preserve the behaviour of inject content automatically without open Popup manually. All code works fine when build in devleopment mode, but in production mode I got the mentioned error.

@aspiers
Copy link
Contributor

aspiers commented Jan 29, 2024

@lealahm @romaindequidt the latest vite preload plugin already solved this problem, 685da06

@tngflx It didn't solve it for me; please can you share a working example?

@tngflx
Copy link

tngflx commented Jan 30, 2024

@aspiers Hi man, can I know what's your error? By running the inline script by default in vite.config.ts that should do it, that plugin is just a hook event inside vite upon bundling.

Edit : ok I take back my word, the inline script on latest commit is rather buggy, the first line

if (!/content/.test(chunk.fileName)) {
              return null;
     }

already f us over...

@aspiers
Copy link
Contributor

aspiers commented Jan 30, 2024

Edit : ok I take back my word, the inline script on latest commit is rather buggy, the first line

if (!/content/.test(chunk.fileName)) {
              return null;
     }

already f us over...

I already provided this solution in this comment.

@tngflx
Copy link

tngflx commented Jan 30, 2024

try my solution... #357

    const filter = createFilter(options.include, options.exclude);
    
async writeBundle(outputOptions, bundle) {
    const outputDir = outputOptions.dir || (outputOptions.file && path.dirname(outputOptions.file));
    let fileToDelete: string | null = null;

    if (!outputDir) {
        console.error('Unable to determine the output directory.');
        return;
    }

    for (const [fileName, outputChunk ] of Object.entries(bundle) as [string, OutputChunk][]) {
        if (!filter(fileName)) {
            continue; // Skip files that don't match the filter
        }

        let modifiedCode = outputChunk.code;

        // Check if the filename contains "index.js"
        if (fileName.includes('index.js')) {
            // Replace import statements with the actual code
            const importStatementRegex = /import\s*\{[^\}]*__vitePreload[^\}]*\}\s*from\s*['"]([^'"]+)['"];/g;
            const exportStatementRegex = /export\s*\{\s*__vitePreload\s*as\s*_\s*\};/g;

            modifiedCode = modifiedCode.replace(importStatementRegex, (_, relativePath) => {
                const absolutePath = path.resolve(outputDir, path.dirname(fileName), relativePath);

                try {
                    let fileContent = fs.readFileSync(absolutePath, 'utf-8');
                    fileContent = fileContent.replace(exportStatementRegex, '');
                    return fileContent;

                } catch (error) {
                    console.error(`Error reading file: ${absolutePath}`, error);
                    return _; // Return the original import statement if there is an error
                }finally {
                    // Store the file path for deletion outside the loop
                    fileToDelete = absolutePath;
                }
            })
        }
        
        try {
            fs.writeFileSync(path.resolve(outputDir, fileName), modifiedCode, 'utf-8');
        } catch (error) {
            console.error(`Error writing file: ${path.resolve(outputDir, fileName)}`, error);
        }

    }

    if (fileToDelete) {
        try {
            fs.unlinkSync(fileToDelete);
        } catch (error) {
            console.error(`Error removing file: ${fileToDelete}`, error);
        }
    }
},

I made this solution before i found the latest commit solution, mine works a bit better and skipped the filename check and only focus on index.js. Its a bit bloated maybe you can rewrite it with renderchunk hook

@Jonghakseo
Copy link
Owner

To be honest, I simply started this boilerplate without full knowledge of Vite and the bundling process of Rollup that it uses internally, and I think this is the time to confront that limitation to some extent.

I'm using a lot of vitePlugins to get things to do what I want, or to fix bugs, and I'm increasingly convinced that a fundamental solution is needed.

And that is to completely tear down this boilerplate and fix it at a lower layer based on rollup.

In the process, I think we can also implement a true HMR (not the inconvenient way we have now, where we throw in a refresh method and a path).

The only limitation is my physical time...

@aspiers
Copy link
Contributor

aspiers commented Feb 18, 2024

@Jonghakseo Makes sense, and I totally understand about the lack of time! So grateful for what you already built here so far though; it enabled me to build my extension which probably would have taken too long for me to do by myself!

@PatrykKuniczak
Copy link
Collaborator

@Jonghakseo Are you want to implement this PR?

@PatrykKuniczak
Copy link
Collaborator

@Jonghakseo One more ping :)

@Jonghakseo
Copy link
Owner

@Jonghakseo One more ping :)

Thanks~! I'll check it soon ^^

@PatrykKuniczak
Copy link
Collaborator

@Jonghakseo One more ping :)

Thanks~! I'll check it soon ^^

If you decide to move forward with that, maybe i can help with that, it have some months, i need to read it 1 more time :)

@PatrykKuniczak
Copy link
Collaborator

@Jonghakseo Let's do something with this thread cause maybe somebody waiting for this feature :)

@aspiers
Copy link
Contributor

aspiers commented Jun 16, 2024

I need this feature 😜 Would be fantastic if a solution was found 🙏

@Jonghakseo Jonghakseo changed the base branch from main to legacy June 16, 2024 13:51
@Jonghakseo
Copy link
Owner

@PatrykKuniczak @aspiers

If I understand correctly, this should be worked on top of the legacy branch, is that correct?

@Jonghakseo
Copy link
Owner

Maybe.... this is solution...?

@PatrykKuniczak
Copy link
Collaborator

Maybe.... this is solution...?

I think so

@PatrykKuniczak PatrykKuniczak mentioned this pull request Jun 16, 2024
Jonghakseo added a commit that referenced this pull request Jul 6, 2024
Co-authored-by: Darwin Swartz <[email protected]>
Co-authored-by: Leal <[email protected]>
Co-authored-by: Romain Dequidt <[email protected]>
Co-authored-by: Jonghakseo <[email protected]>
Co-authored-by: PatrykKuniczak <[email protected]>
@Jonghakseo
Copy link
Owner

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.

Can't add a custom Content Script folder
8 participants