Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_js_formatter): Insert Space after type for import equals declaration #2391

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Apr 12, 2022

Insert a space after the type keyword for import equals declarations.

Insert a space after the `type` keyword for import equals declarations.
@MichaReiser MichaReiser changed the title fix(rome_js_formatter): Insert Space before type for import equals declaration fix(rome_js_formatter): Insert Space after type for import equals declaration Apr 12, 2022
@MichaReiser MichaReiser requested a review from leops April 12, 2022 10:32
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45250 45250 0
Passed 44310 44310 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.92% 97.92% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

ts/babel

Test result main count This PR count Difference
Total 584 584 0
Passed 508 508 0
Failed 76 76 0
Panics 0 0 0
Coverage 86.99% 86.99% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 15983 15983 0
Passed 12167 12167 0
Failed 3816 3816 0
Panics 0 0 0
Coverage 76.12% 76.12% 0.00%

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: e0e4e71
Status: ✅  Deploy successful!
Preview URL: https://148d1a32.tools-8rn.pages.dev

View logs

Comment on lines -32 to -41
import typeA = require("foo");
import type A = require("foo");
export import type = require("A");

import typeA = require("A");
import type A = require("A");

import typea = require("a");
import type a = require("a");

export import typeB = require("B");
export import type B = require("B");

export import typeb = require("b");
Copy link
Contributor

Choose a reason for hiding this comment

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

@MichaReiser Does this mean that this is valid syntax??

Copy link
Contributor Author

@MichaReiser MichaReiser Apr 12, 2022

Choose a reason for hiding this comment

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

export import type B = require("B");

This is valid, yes :D I would point you to the spec if there would be any...

Copy link
Contributor

Choose a reason for hiding this comment

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

No no, I mean, is this export import typeb = require("b"); valid?

Because this is really strange. Our testing suite makes sure that emitted code is actually valid (it parses again) and it seems that it's not doing so... so, there are two possible explanation:

  • the formatter testing suite is not catching this case (it doesn't reparse) [likely]
  • the parse doesn't emit any syntax errors [unlikely]

Copy link
Contributor

Choose a reason for hiding this comment

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

(yeah valid syntax lol, an unfortunate coincidence lol)

Copy link
Contributor Author

@MichaReiser MichaReiser Apr 12, 2022

Choose a reason for hiding this comment

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

Yeah, it's valid because typeb is a valid identifier and the type keyword is optional.

@MichaReiser MichaReiser merged commit be2114f into main Apr 12, 2022
@MichaReiser MichaReiser deleted the fix/import-equals-declaration branch April 12, 2022 13:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants