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

improve performance and fix regex reset .proxy.js imports #37

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dnikku
Copy link

@dnikku dnikku commented Mar 8, 2021

Hi, i improved performance of regex (start line with ^import); In my project, where I use vuetify, on file 'vuetify.css.proxy.js' it took more than 10 minutes ...
and i fixed (make it multiline "/mg") for scenarios when the import "...proxy.js" is not on the first line.

@KonnorRogers
Copy link
Owner

hmm...I think making it multiline isnt a bad idea, I worry about possibly being too open and letting bugs slip in with this implementation. Ideally theres an AST parser to detect imports...anyways, by using /^import/ it'll miss if a user does: await import

@dnikku
Copy link
Author

dnikku commented Mar 8, 2021

AST it is ideal as you said, but not necessary ...
The big problem is the regex goes into "catastrophic backtracking" (see: https://community.appway.com/screen/kb/article/checking-strings-avoiding-catastrophic-backtracking-1482810891360).

a better regex is: /import\s+['"].+.(\w+).proxy.js['"]/mg

ironically the problem occurs in a file like something.ext.proxy.js file, maybe it is better to ignore these files, too!

index.js (line 154)
// Rewrite "proxy.js" imports prior to building
glob
.sync(buildDirectory + "/**/*.js", { nodir: true })
.filter(file => !file.endsWith(".proxy.js")) // Added this filter
...

Snippet from the vuetify.css.proxy.js (has 650KB and only 10 lines): Catastrophic backtracking happend on import(ant) ...

// [snowpack] add styles to the page (skip if no document exists)
if (typeof document !== 'undefined') {
const code = ".theme--light.v-application {\n background: #FFFFFF;\n color: rgba(0, 0, 0, 0.87);\n}\n.theme--light.v-application .text--primary {\n color: rgba(0, 0, 0, 0.87) !important;\n}\n.theme--light.v-application .text--secondary {\n color: rgba(0, 0, 0, 0.6) !important;\n}\n.theme--light.v-application .text--disabled {\n color: rgba(0, 0, 0, 0.38) !important;\n}\n\n.theme--dark.v-application {\n bac ...

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.

2 participants