[lexical][lexical-yjs][lexical-playground] Chore: Respect browserslist#8512
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| // Compatibility with browserslist | ||
| { | ||
| ...compat.configs['flat/recommended'], | ||
| ignores: ['packages/lexical-playground/**'], |
There was a problem hiding this comment.
Do you think compatibility should be enforced in playgrounds? Theoretically, it's possible to set up checks there and disable some code. Or add a fallback for simple cases like structuredClone
> @lexical/monorepo@0.44.0 lint
> eslint ./
./packages/lexical-playground/src/plugins/AutocompletePlugin/index.tsx
93:25 error navigator.userAgentData() is not supported in Safari 15, Firefox 115, Edge 86, Chrome 86 compat/compat
./packages/lexical-playground/src/plugins/ContextMenuPlugin/index.tsx
70:38 error navigator.permissions() is not supported in Safari 15 compat/compat
99:38 error navigator.permissions() is not supported in Safari 15 compat/compat
./packages/lexical-playground/src/plugins/PagesReactExtension/PageSetupDropdown.tsx
34:14 error structuredClone is not supported in Safari 15, Edge 86, Chrome 86 compat/compat
./packages/lexical-playground/src/utils/docSerialization.ts
40:14 error CompressionStream is not supported in Safari 15 compat/compat
61:14 error DecompressionStream is not supported in Safari 15 compat/compat
There was a problem hiding this comment.
It's probably best practice to keep the compat on in the playground and simply enable/disable features accordingly. It doesn't look like we have a real use case for structuredClone, a shallow {...clone} will work just as well in those two call sites. The other situations could be behind typeof guards maybe with warnOnlyOnce usage to put something in the console on very old environments
etrepum
left a comment
There was a problem hiding this comment.
I think the @ is missing from the negative lookbehind transformation
|
|
||
| const URL_REGEX = | ||
| /((https?:\/\/(www\.)?)|(www\.))[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_+.~#?&//=]*)(?<![-.+():%])/; | ||
| /((https?:\/\/(www\.)?)|(www\.))[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b(?:[-a-zA-Z0-9()@:%_+.~#?&//=]*[a-zA-Z0-9_~#?&//=])?/; |
There was a problem hiding this comment.
| /((https?:\/\/(www\.)?)|(www\.))[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b(?:[-a-zA-Z0-9()@:%_+.~#?&//=]*[a-zA-Z0-9_~#?&//=])?/; | |
| /((https?:\/\/(www\.)?)|(www\.))[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b(?:[-a-zA-Z0-9()@:%_+.~#?&//=]*[a-zA-Z0-9@_~#?&//=])?/; |
| it.each([true, false])( | ||
| 'preserves indentation (CompressionStream: %s)', | ||
| async hasCompressionStream => { | ||
| if (!hasCompressionStream) { | ||
| vi.stubGlobal('CompressionStream', undefined); | ||
| } |
There was a problem hiding this comment.
What is the point of this? There's no CompressionStream related code anywhere in here?
| encoded = encodeURIComponent(json); | ||
| CompressionAPIWarning(); |
There was a problem hiding this comment.
If we provide a fallback encoding it should decode correctly in implementations that do have the DecompressionStream API. Probably better to just provide a warning and do nothing.
| CompressionAPIWarning(); | ||
| return JSON.parse(decodeURIComponent(b64)); |
There was a problem hiding this comment.
This is primarily going to fail so probably not worth trying
| if (typeof CompressionStream !== 'undefined') { | ||
| const cs = new CompressionStream('gzip'); | ||
| const writer = cs.writable.getWriter(); | ||
| const [, output] = await Promise.all([ | ||
| writer | ||
| .write(new TextEncoder().encode(JSON.stringify(doc))) | ||
| .then(() => writer.close()), | ||
| readBytestoString(cs.readable.getReader()), | ||
| ]); | ||
| return `#doc=${btoa(output) | ||
| .replace(/\//g, '_') | ||
| .replace(/\+/g, '-') | ||
| .replace(/=+$/, '')}`; | ||
| } | ||
|
|
||
| CompressionAPIWarning(); | ||
| return ''; |
There was a problem hiding this comment.
I would generally reverse the order of this, bail early if something is missing and leave the happy path with less indentation
| if (typeof CompressionStream !== 'undefined') { | ||
| const ds = new DecompressionStream('gzip'); | ||
| const writer = ds.writable.getWriter(); | ||
| const b64 = atob(m[1].replace(/_/g, '/').replace(/-/g, '+')); | ||
| const array = new Uint8Array(b64.length); | ||
| for (let i = 0; i < b64.length; i++) { | ||
| array[i] = b64.charCodeAt(i); | ||
| } | ||
| const closed = writer.write(array).then(() => writer.close()); | ||
| const output = []; | ||
| for await (const chunk of generateReader( | ||
| ds.readable.pipeThrough(new TextDecoderStream()).getReader(), | ||
| )) { | ||
| output.push(chunk); | ||
| } | ||
| await closed; | ||
| return JSON.parse(output.join('')); | ||
| } | ||
| await closed; | ||
| return JSON.parse(output.join('')); | ||
| CompressionAPIWarning(); | ||
| return null; |
There was a problem hiding this comment.
should also re-order this
etrepum
left a comment
There was a problem hiding this comment.
this is what I had meant, there's no need for an else here
Co-authored-by: Bob Ippolito <bob@redivi.com>
| function getCompressionStream() { | ||
| if (typeof CompressionStream !== 'undefined') { | ||
| return new CompressionStream('gzip'); | ||
| } | ||
| } |
Description
The project has added an eslint plugin that checks features against the current browserslist https://lexical.dev/docs/getting-started/supported-browsers. This will help avoid potential compatibility issues
Closes #6573
Test plan
Before
There is no rule that would automatically check the compatibility of a feature with a specific list of browsers.
After
Object.hasOwnhas been replaced with the old safe equivalentObject.prototype.hasOwnProperty