Skip to content

fixture: watch cannot build when files are changed or added successfully#2457

Merged
1 commit merged into
masterfrom
unknown repository
Aug 29, 2019
Merged

fixture: watch cannot build when files are changed or added successfully#2457
1 commit merged into
masterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Aug 24, 2019

  1. For function 'fullBuild', the first statement 'opts' can be divided into two parameters called "selectedLocales" and "reserveLocale", the order isn't right, according to the caller of this function itself (See its calling at server.js by 'build.fullBuild' and other related functions). So switch them together.

  2. Fix watch is broken #2443 (comment) for watch, because we didn't pass the parameter into the buildLocale, there'll be Unhandled Exception.

@XhmikosR
Copy link
Copy Markdown
Contributor

@MaledongGit Please see #2443 (comment)

Something is wrong anyway in serve.js. IMO we should adjust serve.js.

@XhmikosR
Copy link
Copy Markdown
Contributor

Watch is broken not build. Just try editing a markdown file from any locale.

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 25, 2019

@XhmikosR , OK, In fact I've fixed it locally, but since you've mentioned me by this, I'll submit mine here;)

@ghost ghost changed the title fixture: switch the parameters fixture: watch cannot build files changed or added successfully Aug 25, 2019
@ghost ghost changed the title fixture: watch cannot build files changed or added successfully fixture: watch cannot build when files are changed or added successfully Aug 25, 2019
@ghost ghost requested review from MylesBorins and Trott August 25, 2019 02:26
@ghost ghost mentioned this pull request Aug 25, 2019
@ghost ghost added the bug label Aug 25, 2019
@XhmikosR
Copy link
Copy Markdown
Contributor

@MaledongGit: your patch works when there's one locale set in DEFAULT_LOCALE. When there's more than one, it doesn't work properly.

I'd say, drop this patch from here and let's tackle it in a separate branch. We probably need to export generateLocalesData in build.js and use this in server.js.

@XhmikosR
Copy link
Copy Markdown
Contributor

When I don't set a default locale, and I change for example an English md file, after rebuild, I get this in the browser console:

TypeError: currentLangElement is null
en:116:22

It might not be related to this patch, though.

That being said, with your patch, if I have DEFAULT_LOCALE=en and I change a German content file for example, I see the console message that it changed but it's useless since we aren't watching for files other than DEFAULT_LOCALE. Maybe remove your console messages completely.

@ghost
Copy link
Copy Markdown
Author

ghost commented Aug 25, 2019

@XhmikosR, No problem, I'll repost it and give you the feedback of my result, still say "A BIG THANKYOU!"

@ghost ghost requested a review from alexandrtovmach August 25, 2019 09:20
For function 'fullBuild', the first statement 'opts' can be divided into
two parameters called "selectedLocales" and "reserveLocale", the order
isn't right, according to the caller of this function itself (See its
calling at server.js by 'build.fullBuild' and other related functions).
So switch them together.

Fix #2443 (comment) for watch, because we didn't pass the parameter into
the buildLocale, there'll be Unhandled Exception.
@ghost ghost merged commit 865a89d into nodejs:master Aug 29, 2019
This pull request was closed.
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.

watch is broken

1 participant