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.

Sign up for GitHub

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

Updates our sync property access for params and searchParams to allow value #70570

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Updates our sync property access for params and searchParams to allow value #70570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions packages/next/src/server/request/params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ function makeAbortingExoticParams(

// React Promise extension
// fallthrough
case 'value':
case 'status':

// Common tested properties
Expand Down Expand Up @@ -284,7 +283,6 @@ function makeErroringExoticParams(

// React Promise extension
// fallthrough
case 'value':
case 'status':

// Common tested properties
Expand Down Expand Up @@ -394,7 +392,6 @@ function makeUntrackedExoticParams(underlyingParams: Params): Promise<Params> {

// React Promise extension
// fallthrough
case 'value':
case 'status':

// Common tested properties
Expand Down Expand Up @@ -450,7 +447,6 @@ function makeDynamicallyTrackedExoticParamsWithDevWarnings(

// React Promise extension
// fallthrough
case 'value':
case 'status':

// Common tested properties
Expand Down Expand Up @@ -483,6 +479,12 @@ function makeDynamicallyTrackedExoticParamsWithDevWarnings(
}
return ReflectAdapter.get(target, prop, receiver)
},
set(target, prop, value, receiver) {
if (typeof prop === 'string') {
proxiedProperties.delete(prop)
}
return ReflectAdapter.set(target, prop, value, receiver)
},
ownKeys(target) {
warnForEnumeration(store.route, unproxiedProperties)
return Reflect.ownKeys(target)
Expand Down
16 changes: 6 additions & 10 deletions packages/next/src/server/request/search-params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,6 @@ function makeAbortingExoticSearchParams(
case 'catch':
case 'finally':

// React Promise extension
// fallthrough
case 'value':

// Common tested properties
// fallthrough
case 'toJSON':
Expand Down Expand Up @@ -307,10 +303,6 @@ function makeErroringExoticSearchParams(
case 'catch':
case 'finally':

// React Promise extension
// fallthrough
case 'value':

// Common tested properties
// fallthrough
case 'toJSON':
Expand Down Expand Up @@ -471,7 +463,6 @@ function makeUntrackedExoticSearchParams(

// React Promise extension
// fallthrough
case 'value':
case 'status':

// Common tested properties
Expand Down Expand Up @@ -593,7 +584,6 @@ function makeDynamicallyTrackedExoticSearchParamsWithDevWarnings(

// React Promise extension
// fallthrough
case 'value':
case 'status':

// Common tested properties
Expand Down Expand Up @@ -641,6 +631,12 @@ function makeDynamicallyTrackedExoticSearchParamsWithDevWarnings(
}
return ReflectAdapter.get(target, prop, receiver)
},
set(target, prop, value, receiver) {
if (typeof prop === 'string') {
proxiedProperties.delete(prop)
}
return Reflect.set(target, prop, value, receiver)
},
has(target, prop) {
if (typeof prop === 'string') {
const expression = describeHasCheckingStringProperty(
Expand Down
36 changes: 22 additions & 14 deletions test/e2e/app-dir/dynamic-io/dynamic-io.params.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2351,7 +2351,7 @@ describe('dynamic-io', () => {
}
})

it('should not allow param names like then, value, and status when accessing params directly in a server component', async () => {
it('should not allow param names like then and status when accessing params directly in a server component', async () => {
expect(getLines('In route /params')).toEqual([])
let $ = await next.render$(
'/params/shadowing/foo/bar/baz/qux/sync/layout/server'
Expand All @@ -2363,14 +2363,16 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([
expect.stringContaining(
'The following properties were not copied: `then`, `value`, , and `status`.'
'The following properties were not copied: `then` and `status`.'
),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
])
} else {
expect($('#layout').text()).toBe('at runtime')
Expand All @@ -2379,7 +2381,7 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([])
}
Expand All @@ -2394,14 +2396,16 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([
expect.stringContaining(
'The following properties were not copied: `then`, `value`, , and `status`.'
'The following properties were not copied: `then` and `status`.'
),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
])
} else {
expect($('#layout').text()).toBe('at runtime')
Expand All @@ -2410,13 +2414,13 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([])
}
})

it('should not allow param names like then, value, and status when accessing params directly in a client component', async () => {
it('should not allow param names like then and status when accessing params directly in a client component', async () => {
expect(getLines('In route /params')).toEqual([])
let $ = await next.render$(
'/params/shadowing/foo/bar/baz/qux/sync/layout/client'
Expand All @@ -2428,14 +2432,16 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([
expect.stringContaining(
'The following properties were not copied: `then`, `value`, , and `status`.'
'The following properties were not copied: `then` and `status`.'
),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
])
} else {
expect($('#layout').text()).toBe('at runtime')
Expand All @@ -2444,7 +2450,7 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([])
}
Expand All @@ -2459,14 +2465,16 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([
expect.stringContaining(
'The following properties were not copied: `then`, `value`, , and `status`.'
'The following properties were not copied: `then` and `status`.'
),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
expect.stringContaining('accessed directly with `params.dyn`'),
expect.stringContaining('accessed directly with `params.value`'),
])
} else {
expect($('#layout').text()).toBe('at runtime')
Expand All @@ -2475,7 +2483,7 @@ describe('dynamic-io', () => {
expect($('#param-then').text()).toEqual(
expect.stringContaining('native code')
)
expect($('#param-value').text()).toBe('undefined')
expect($('#param-value').text()).toBe('baz')
expect($('#param-status').text()).toBe('undefined')
expect(getLines('In route /params')).toEqual([])
}
Expand Down
36 changes: 28 additions & 8 deletions test/e2e/app-dir/dynamic-io/dynamic-io.search.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,10 @@ describe('dynamic-io', () => {
let searchWarnings = getLines('In route /search')
if (isNextDev) {
expect($('#layout').text()).toBe('at runtime')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
expect(searchWarnings).toEqual([
expect.stringContaining(
Expand All @@ -400,6 +401,9 @@ describe('dynamic-io', () => {
'accessed directly with `searchParams.sentinel`'
),
expect.stringContaining('accessed directly with `searchParams.foo`'),
expect.stringContaining(
'accessed directly with `searchParams.value`'
),
])
} else {
expect(searchWarnings).toHaveLength(0)
Expand All @@ -408,9 +412,10 @@ describe('dynamic-io', () => {
// This test case aborts synchronously and the later component render
// triggers the outer boundary
expect($('main').text()).toContain('outer loading...')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
}

Expand Down Expand Up @@ -450,9 +455,10 @@ describe('dynamic-io', () => {
let searchWarnings = getLines('In route /search')
if (isNextDev) {
expect($('#layout').text()).toBe('at runtime')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
expect(searchWarnings).toEqual([
expect.stringContaining(
Expand All @@ -462,15 +468,19 @@ describe('dynamic-io', () => {
'accessed directly with `searchParams.sentinel`'
),
expect.stringContaining('accessed directly with `searchParams.foo`'),
expect.stringContaining(
'accessed directly with `searchParams.value`'
),
])
} else {
expect(searchWarnings).toHaveLength(0)
expect($('#layout').text()).toBe('at buildtime')
expect($('main').text()).toContain('inner loading...')
expect($('main').text()).not.toContain('outer loading...')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
}

Expand Down Expand Up @@ -690,9 +700,10 @@ describe('dynamic-io', () => {
let searchWarnings = getLines('In route /search')
if (isNextDev) {
expect($('#layout').text()).toBe('at runtime')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
expect(searchWarnings).toEqual([
expect.stringContaining(
Expand All @@ -702,13 +713,17 @@ describe('dynamic-io', () => {
'accessed directly with `searchParams.sentinel`'
),
expect.stringContaining('accessed directly with `searchParams.foo`'),
expect.stringContaining(
'accessed directly with `searchParams.value`'
),
])
} else {
expect(searchWarnings).toHaveLength(0)
expect($('#layout').text()).toBe('at runtime')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
}

Expand Down Expand Up @@ -748,9 +763,10 @@ describe('dynamic-io', () => {
let searchWarnings = getLines('In route /search')
if (isNextDev) {
expect($('#layout').text()).toBe('at runtime')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
expect(searchWarnings).toEqual([
expect.stringContaining(
Expand All @@ -760,13 +776,17 @@ describe('dynamic-io', () => {
'accessed directly with `searchParams.sentinel`'
),
expect.stringContaining('accessed directly with `searchParams.foo`'),
expect.stringContaining(
'accessed directly with `searchParams.value`'
),
])
} else {
expect(searchWarnings).toHaveLength(0)
expect($('#layout').text()).toBe('at runtime')
expect($('[data-value]').length).toBe(2)
expect($('[data-value]').length).toBe(3)
expect($('#value-sentinel').text()).toBe('hello')
expect($('#value-foo').text()).toBe('foo')
expect($('#value-value').text()).toBe('baz')
expect($('#page').text()).toBe('at runtime')
}

Expand Down
Loading