Skip to content

Fix export default regexs - #1799

Merged
lukastaegert merged 1 commit into
rollup:release-0.53from
nicolo-ribaudo:export-default-regexs
Dec 18, 2017
Merged

Fix export default regexs#1799
lukastaegert merged 1 commit into
rollup:release-0.53from
nicolo-ribaudo:export-default-regexs

Conversation

@nicolo-ribaudo

@nicolo-ribaudo nicolo-ribaudo commented Dec 16, 2017

Copy link
Copy Markdown
Contributor

Fixes #1798

I updated the regular expressions to match comments, async functions and generators.
They are built at runtime to make them more readable; they are built only once so the performance overhead shouldn't be noticeable.

The built regexes are these:

/^(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)*export(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)+default(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)*/
/^(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)*export(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)+default(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)+(?:(?:async(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)+)?function(?:(?:\s|\/\/.*[\n\r]|\/\*[^]*?\*\/)*\*)?|class)/

/* jsjjddjksj
*/ // Too many comments lol

async /* [no LineTerminator here] */ function

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

acorn@5.2.1 parse of this source file yields:

Unexpected token (13:37)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nevermind - I forgot the --ecma2018 flag.

@adrianheine

Copy link
Copy Markdown
Contributor

Note that Acorn doesn't parse async iterators yet (since they are stage3), but I wrote a plugin for it.

@nicolo-ribaudo

Copy link
Copy Markdown
Contributor Author

I didn't write a test for async generators, but that regex should work for them.

@lukastaegert lukastaegert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Although complex regexes are always a little dangerous I think your approach makes it possible to at least understand and more importantly modify what is happening here with some confidence, so I think this is good to go.

Maybe in the future we find a way to use more of the AST here (i.e. acorn already did the hard work of parsing for us so maybe we can reuse more of what is already known?). Nevertheless I plan on putting this into the next release as is.

@lukastaegert lukastaegert added this to the 0.53.0 milestone Dec 18, 2017
@lukastaegert
lukastaegert changed the base branch from master to release-0.53 December 18, 2017 18:32
@lukastaegert
lukastaegert merged commit 60b518c into rollup:release-0.53 Dec 18, 2017
@nicolo-ribaudo
nicolo-ribaudo deleted the export-default-regexs branch December 18, 2017 18:35
@lukastaegert lukastaegert modified the milestones: 0.53.0, 0.52.x Dec 19, 2017
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.

4 participants