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

perf: don't manipulate an empty value #25647

Merged
merged 45 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
624ad48
perf(nuxt): don't iterate over an empty string
GalacticHypernova Feb 5, 2024
4a6cee3
Update nuxt.ts
GalacticHypernova Feb 5, 2024
45f034e
Update names.ts
GalacticHypernova Feb 5, 2024
313379f
Update compatibility.ts
GalacticHypernova Feb 5, 2024
19019cf
Update names.ts
GalacticHypernova Feb 5, 2024
7b3714e
fix: reverse order of operation
GalacticHypernova Feb 5, 2024
ded3ff2
fix: use rawVersion
GalacticHypernova Feb 5, 2024
3563b84
Merge branch 'main' into patch-9
GalacticHypernova Feb 9, 2024
90fe0cc
Update warmup.ts
GalacticHypernova Feb 9, 2024
94573d2
Update dev-only.ts
GalacticHypernova Feb 9, 2024
da99310
Update tree-shake.ts
GalacticHypernova Feb 9, 2024
d2bbad5
Update dev-only.ts
GalacticHypernova Feb 9, 2024
6b534c7
Update warmup.ts
GalacticHypernova Feb 9, 2024
28412d8
Update tree-shake.ts
GalacticHypernova Feb 9, 2024
2315d99
Update warmup.ts
GalacticHypernova Feb 9, 2024
da88f27
Update tree-shake.ts
GalacticHypernova Feb 9, 2024
40985b1
Update dev-only.ts
GalacticHypernova Feb 9, 2024
6d6a526
Update module.ts
GalacticHypernova Feb 9, 2024
78d31e8
Update ssr-styles.ts
GalacticHypernova Feb 9, 2024
bf6b815
Update nitro.ts
GalacticHypernova Feb 9, 2024
22480ba
Update components.ts
GalacticHypernova Feb 9, 2024
89df552
Update nuxt-error-page.vue
GalacticHypernova Feb 9, 2024
e3e3245
Update tree-shake.ts
GalacticHypernova Feb 9, 2024
1b506d6
Update dev-only.ts
GalacticHypernova Feb 9, 2024
5f00c81
Update names.ts
GalacticHypernova Feb 9, 2024
345e7c5
Update warmup.ts
GalacticHypernova Feb 9, 2024
2d40294
Update nitro.ts
GalacticHypernova Feb 9, 2024
fbcb8a2
Update module.ts
GalacticHypernova Feb 9, 2024
3a4a98b
Update module.ts
GalacticHypernova Feb 9, 2024
9958378
Update module.ts
GalacticHypernova Feb 9, 2024
e2115fa
Update packages/nuxt/src/core/plugins/dev-only.ts
GalacticHypernova Feb 10, 2024
35e38a7
Merge branch 'main' into patch-9
GalacticHypernova Feb 13, 2024
6dc63dd
perf: app.ts
GalacticHypernova Feb 13, 2024
5481a6b
fix: proper null checks
GalacticHypernova Feb 13, 2024
18435de
chore: reverse
GalacticHypernova Feb 13, 2024
152a082
fix: proper null checks
GalacticHypernova Feb 13, 2024
a134c73
Update payload.ts
GalacticHypernova Feb 13, 2024
caefadf
Merge branch 'main' into patch-9
GalacticHypernova Feb 18, 2024
0697051
chore: minor formatting and refactors
danielroe Feb 25, 2024
7346e73
Merge remote-tracking branch 'origin/main' into patch-9
danielroe Feb 25, 2024
f083066
chore: space
danielroe Feb 25, 2024
a746111
refactor: rename variable
danielroe Feb 25, 2024
0d70ae0
fix: add back missing return
danielroe Feb 25, 2024
12a6d7b
fix: remove unneeded check
danielroe Feb 25, 2024
5edd066
test: sort links
danielroe Feb 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/kit/src/compatibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ export function isNuxt3 (nuxt: Nuxt = useNuxt()) {
* Get nuxt version
*/
export function getNuxtVersion (nuxt: Nuxt | any = useNuxt() /* TODO: LegacyNuxt */) {
const version = (nuxt?._version || nuxt?.version || nuxt?.constructor?.version || '').replace(/^v/g, '')
if (!version) {
const rawVersion = nuxt?._version || nuxt?.version || nuxt?.constructor?.version
if (!rawVersion) {
throw new Error('Cannot determine nuxt version! Is current instance passed?')
}
return version
return rawVersion.replace(/^v/g, '')
}
2 changes: 1 addition & 1 deletion packages/kit/src/loader/nuxt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export async function loadNuxt (opts: LoadNuxtOptions): Promise<Nuxt> {
throw new Error(`Cannot find any nuxt version from ${opts.cwd}`)
}
const pkg = await readPackageJSON(nearestNuxtPkg)
const majorVersion = parseInt((pkg.version || '').split('.')[0])
const majorVersion = pkg.version ? parseInt(pkg.version.split('.')[0]) : ''

const rootDir = pathToFileURL(opts.cwd || process.cwd()).href

Expand Down
32 changes: 17 additions & 15 deletions packages/nuxt/src/app/components/nuxt-error-page.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,23 @@ const props = defineProps({
const _error = props.error

// TODO: extract to a separate utility
const stacktrace = (_error.stack || '')
.split('\n')
.splice(1)
.map((line) => {
const text = line
.replace('webpack:/', '')
.replace('.vue', '.js') // TODO: Support sourcemap
.trim()
return {
text,
internal: (line.includes('node_modules') && !line.includes('.cache')) ||
line.includes('internal') ||
line.includes('new Promise')
}
}).map(i => `<span class="stack${i.internal ? ' internal' : ''}">${i.text}</span>`).join('\n')
const stacktrace = _error.stack
? _error.stack
.split('\n')
.splice(1)
.map((line) => {
const text = line
.replace('webpack:/', '')
.replace('.vue', '.js') // TODO: Support sourcemap
.trim()
return {
text,
internal: (line.includes('node_modules') && !line.includes('.cache')) ||
line.includes('internal') ||
line.includes('new Promise')
}
}).map(i => `<span class="stack${i.internal ? ' internal' : ''}">${i.text}</span>`).join('\n')
: ''

// Error page props
const statusCode = Number(_error.statusCode || 500)
Expand Down
2 changes: 1 addition & 1 deletion packages/nuxt/src/core/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@
for (const plugin of _plugins) {
// Make sure dependency plugins are registered
if (plugin.dependsOn && plugin.dependsOn.some(name => !pluginNames.includes(name))) {
console.error(`Plugin \`${plugin.name}\` depends on \`${plugin.dependsOn.filter(name => !pluginNames.includes(name)).join(', ')}\` but they are not registered.`)

Check warning on line 223 in packages/nuxt/src/core/app.ts

View workflow job for this annotation

GitHub Actions / code

Unexpected console statement
}
// Make graph to detect circular dependencies
if (plugin.name) {
Expand All @@ -229,11 +229,11 @@
}
const checkDeps = (name: string, visited: string[] = []): string[] => {
if (visited.includes(name)) {
console.error(`Circular dependency detected in plugins: ${visited.join(' -> ')} -> ${name}`)

Check warning on line 232 in packages/nuxt/src/core/app.ts

View workflow job for this annotation

GitHub Actions / code

Unexpected console statement
return []
}
visited.push(name)
return (deps[name] || []).flatMap(dep => checkDeps(dep, [...visited]))
return deps[name]?.length ? deps[name].flatMap(dep => checkDeps(dep, [...visited])) : []
}
for (const name in deps) {
checkDeps(name)
Expand Down
16 changes: 9 additions & 7 deletions packages/nuxt/src/core/nitro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,13 +287,15 @@ export async function initNitro (nuxt: Nuxt & { _nitro?: Nitro }) {
const routeRulesMatcher = toRouteMatcher(
createRadixRouter({ routes: routeRules })
)
const payloadSuffix = nuxt.options.experimental.renderJsonPayloads ? '/_payload.json' : '/_payload.js'
for (const route of nitro._prerenderedRoutes || []) {
if (!route.error && route.route.endsWith(payloadSuffix)) {
const url = route.route.slice(0, -payloadSuffix.length) || '/'
const rules = defu({}, ...routeRulesMatcher.matchAll(url).reverse()) as Record<string, any>
if (!rules.prerender) {
prerenderedRoutes.add(url)
if (nitro._prerenderedRoutes?.length) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't see an issue with the previous code here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is kind of the same principle as everything else, if the array is empty the performance overhead to begin checking the iteration (even if exiting immediately) is much bigger than checking for a truthy length. I'll be able to append benchmark results when I get on PC but there was a notable increase in performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: here it is.

const routeRulesMatcher=""
function defu(table, ...args){

}
const prerenderedRoutes=new Set()
const payloadSuffix=""
const nitro={
    _prerenderedRoutes:[]
}
console.time("current")
for (let index = 0; index < 100000; index++) {

    for (const route of nitro._prerenderedRoutes || []) {
        if (!route.error && route.route.endsWith(payloadSuffix)) {
          const url = route.route.slice(0, -payloadSuffix.length) || '/'
          const rules = defu({}, ...routeRulesMatcher.matchAll(url).reverse())
          if (!rules.prerender) {
            prerenderedRoutes.add(url)
          }
        }
    }
}
console.timeEnd("current")
console.time("PR")
for (let index = 0; index < 100000; index++) {
    if (nitro._prerenderedRoutes?.length) {
        for (const route of nitro._prerenderedRoutes) {
          if (!route.error && route.route.endsWith(payloadSuffix)) {
            const url = route.route.slice(0, -payloadSuffix.length) || '/'
            const rules = defu({}, ...routeRulesMatcher.matchAll(url).reverse())
            if (!rules.prerender) {
              prerenderedRoutes.add(url)
            }
          }
        }
    }
}

console.timeEnd("PR")

image

const payloadSuffix = nuxt.options.experimental.renderJsonPayloads ? '/_payload.json' : '/_payload.js'
for (const route of nitro._prerenderedRoutes) {
if (!route.error && route.route.endsWith(payloadSuffix)) {
const url = route.route.slice(0, -payloadSuffix.length) || '/'
const rules = defu({}, ...routeRulesMatcher.matchAll(url).reverse()) as Record<string, any>
if (!rules.prerender) {
prerenderedRoutes.add(url)
}
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions packages/nuxt/src/core/plugins/dev-only.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ export const DevOnlyPlugin = createUnplugin((options: DevOnlyPluginOptions) => {
if (!DEVONLY_COMP_SINGLE_RE.test(code)) { return }

const s = new MagicString(code)
for (const match of code.matchAll(DEVONLY_COMP_RE) || []) {
for (const match of code.matchAll(DEVONLY_COMP_RE)) {
const ast: Node = parse(match[0]).children[0]
const fallback: Node | undefined = ast.children?.find((n: Node) => n.name === 'template' && Object.values(n.attributes).includes('#fallback'))
const replacement = fallback ? match[0].slice(fallback.loc[0].end, fallback.loc[fallback.loc.length - 1].start) : ''

s.overwrite(match.index!, match.index! + match[0].length, replacement)
}

Expand Down
2 changes: 1 addition & 1 deletion packages/nuxt/src/core/plugins/tree-shake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const TreeShakeComposablesPlugin = createUnplugin((options: TreeShakeComp

const s = new MagicString(code)
const strippedCode = stripLiteral(code)
for (const match of strippedCode.matchAll(COMPOSABLE_RE_GLOBAL) || []) {
for (const match of strippedCode.matchAll(COMPOSABLE_RE_GLOBAL)) {
s.overwrite(match.index!, match.index! + match[0].length, `${match[1]} false && /*@__PURE__*/ ${match[2]}`)
}

Expand Down
2 changes: 1 addition & 1 deletion packages/nuxt/src/core/utils/names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function resolveComponentNameSegments (fileName: string, prefixParts: str
let index = prefixParts.length - 1
const matchedSuffix: string[] = []
while (index >= 0) {
matchedSuffix.unshift(...splitByCase(prefixParts[index] || '').map(p => p.toLowerCase()))
matchedSuffix.unshift(...splitByCase(prefixParts[index]).map(p => p.toLowerCase()))
const matchedSuffixContent = matchedSuffix.join('/')
if ((fileNamePartsContent === matchedSuffixContent || fileNamePartsContent.startsWith(matchedSuffixContent + '/')) ||
// e.g Item/Item/Item.vue -> Item
Expand Down
8 changes: 4 additions & 4 deletions packages/nuxt/src/head/runtime/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ export const NoScript = defineComponent({
},
setup: setupForUseMeta((props, { slots }) => {
const noscript = { ...props }
const textContent = (slots.default?.() || [])
.filter(({ children }) => children)
.map(({ children }) => children)
.join('')
const slotVnodes = slots.default?.()
const textContent = slotVnodes
? slotVnodes.filter(({ children }) => children).map(({ children }) => children).join('')
: ''
if (textContent) {
noscript.children = textContent
}
Expand Down
18 changes: 10 additions & 8 deletions packages/nuxt/src/pages/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,13 +271,15 @@ export default defineNuxtModule({
processPages(pages)
})
nuxt.hook('nitro:build:before', (nitro) => {
for (const route of nitro.options.prerender.routes || []) {
// Skip default route value as we only generate it if it is already
// in the detected routes from `~/pages`.
if (route === '/') { continue }
prerenderRoutes.add(route)
if (nitro.options.prerender.routes.length) {
for (const route of nitro.options.prerender.routes) {
// Skip default route value as we only generate it if it is already
// in the detected routes from `~/pages`.
if (route === '/') { continue }
prerenderRoutes.add(route)
}
nitro.options.prerender.routes = Array.from(prerenderRoutes)
}
nitro.options.prerender.routes = Array.from(prerenderRoutes)
})
})

Expand Down Expand Up @@ -389,13 +391,13 @@ export default defineNuxtModule({
const getSources = (pages: NuxtPage[]): string[] => pages
.filter(p => Boolean(p.file))
.flatMap(p =>
[relative(nuxt.options.srcDir, p.file as string), ...getSources(p.children || [])]
[relative(nuxt.options.srcDir, p.file as string), ...(p.children?.length ? getSources(p.children) : [])]
)

// Do not prefetch page chunks
nuxt.hook('build:manifest', (manifest) => {
if (nuxt.options.dev) { return }
const sourceFiles = getSources(nuxt.apps.default.pages || [])
const sourceFiles = nuxt.apps.default?.pages?.length ? getSources(nuxt.apps.default.pages) : []

for (const key in manifest) {
if (manifest[key].isEntry) {
Expand Down
2 changes: 2 additions & 0 deletions packages/vite/src/plugins/ssr-styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ export function ssrStylesPlugin (options: SSRStylePluginOptions): Plugin {
if (id === options.entry && (options.shouldInline === true || (typeof options.shouldInline === 'function' && options.shouldInline(id)))) {
const s = new MagicString(code)
options.clientCSSMap[id] ||= new Set()
if (!options.globalCSS.length) { return }

for (const file of options.globalCSS) {
const resolved = await this.resolve(file) ?? await this.resolve(file, id)
const res = await this.resolve(file + '?inline&used') ?? await this.resolve(file + '?inline&used', id)
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/utils/warmup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export async function warmupViteServer (

try {
const mod = await server.moduleGraph.getModuleByUrl(url, isServer)
const deps = mod?.ssrTransformResult?.deps /* server */ || Array.from(mod?.importedModules /* client */ || []).map(m => m.url)
const deps = mod?.ssrTransformResult?.deps /* server */ || mod?.importedModules.size ? Array.from(mod?.importedModules /* client */).map(m => m.url) : []
await Promise.all(deps.map(m => warmup(m)))
} catch (e) {
logger.debug('[warmup] tracking dependencies for %s failed with: %s', url, e)
Expand Down
1 change: 1 addition & 0 deletions test/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,7 @@ describe('component islands', () => {
link.href = link.href.replace(fixtureDir, '/<rootDir>').replaceAll('//', '/')
link.key = link.key.replace(/-[a-zA-Z0-9]+$/, '')
}
result.head.link.sort((a, b) => b.href.localeCompare(a.href))
}

// TODO: fix rendering of styles in webpack
Expand Down